Skip to content

Commit

Permalink
fix: golangci.yml to keep quality of codes (#1296)
Browse files Browse the repository at this point in the history
* fix: invalid golanci.yml format

* fix: apply golangci

* fix: apply make docs

* fix: bad expressions detected with golangci-lint

Co-authored-by: Scott Winkler <scott.winkler@snowflake.com>
  • Loading branch information
mnagaa and sfc-gh-swinkler authored Oct 30, 2022
1 parent 526b852 commit 792665f
Show file tree
Hide file tree
Showing 50 changed files with 125 additions and 130 deletions.
25 changes: 18 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
run:
linters:
enable:
- gofmt
- whitespace
- unparam
- scopelint
- godot
timeout: 5m
skip-files: []
# skip-dirs:

linters-settings:
misspell:
locale: US

linters:
disable-all: true
enable:
- gofmt
- whitespace
# - unparam
- exportloopref
- godot
# - gocritic
- misspell
1 change: 0 additions & 1 deletion pkg/datasources/databases.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ func ReadDatabases(d *schema.ResourceData, meta interface{}) error {
}
}
databases = append(databases, dbR)

}
databasesErr := d.Set("databases", databases)
if databasesErr != nil {
Expand Down
5 changes: 2 additions & 3 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ func DSN(
host string,
protocol string,
port int,
warehouse string) (string, error) {

warehouse string,
) (string, error) {
// us-west-2 is Snowflake's default region, but if you actually specify that it won't trigger the default code
// https://github.com/snowflakedb/gosnowflake/blob/52137ce8c32eaf93b0bd22fc5c7297beff339812/dsn.go#L61
if region == "us-west-2" {
Expand Down Expand Up @@ -503,7 +503,6 @@ func GetOauthAccessToken(
clientID,
clientSecret string,
data url.Values) (string, error) {

client := &http.Client{}
request, err := GetOauthRequest(strings.NewReader(data.Encode()), endPoint, clientID, clientSecret)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func TestDSN(t *testing.T) {
"user4:pass4@zha1234.us-east-1.privatelink.snowflakecomputing.com:8443?account=acct4&application=terraform-provider-snowflake&ocspFailOpen=true&role=role4&validateDefaultParameters=true", false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := provider.DSN(tt.args.account, tt.args.user, tt.args.password, tt.args.browserAuth, "", "", "", "", tt.args.region, tt.args.role, tt.args.host, tt.args.protocol, tt.args.port, "")
if (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -90,6 +91,7 @@ func TestOAuthDSN(t *testing.T) {
"", true},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := provider.DSN(tt.args.account, tt.args.user, "", false, "", "", "", tt.args.oauthAccessToken, tt.args.region, tt.args.role, tt.args.host, tt.args.protocol, tt.args.port, "")

Expand Down Expand Up @@ -125,6 +127,7 @@ func TestGetOauthDATA(t *testing.T) {
false},
}
for _, tt := range cases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got := provider.GetOauthData(tt.param.refreshToken, tt.param.redirectURL)
want, err := url.ParseQuery(tt.want)
Expand All @@ -135,7 +138,6 @@ func TestGetOauthDATA(t *testing.T) {
if !reflect.DeepEqual(got, want) {
t.Errorf("GetData() = %v, want %v", got, tt.want)
}

})
}
}
Expand All @@ -162,6 +164,7 @@ func TestGetOauthResponse(t *testing.T) {
false},
}
for _, tt := range cases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := provider.GetOauthRequest(strings.NewReader(tt.param.dataStuff), tt.param.endpoint, tt.param.clientid, tt.param.clientscret)
if err != nil {
Expand Down Expand Up @@ -216,6 +219,7 @@ func TestGetOauthAccessToken(t *testing.T) {
"404", "", false},
}
for _, tt := range cases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
client := NewTestClient(func(req *http.Request) *http.Response {
// Test request parameters
Expand Down
4 changes: 2 additions & 2 deletions pkg/resources/database_grant_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func testRolesAndShares(t *testing.T, path string, roles, shares []string) func(*terraform.State) error {
func testRolesAndShares(t *testing.T, path string, roles []string) func(*terraform.State) error {
t.Helper()
return func(state *terraform.State) error {
is := state.RootModule().Resources[path].Primary
Expand Down Expand Up @@ -49,7 +49,7 @@ func TestAcc_DatabaseGrant(t *testing.T) {
resource.TestCheckResourceAttr("snowflake_database_grant.test", "roles.#", "1"),
resource.TestCheckResourceAttr("snowflake_database_grant.test", "shares.#", "1"),
resource.TestCheckResourceAttr("snowflake_database_grant.test", "shares.#", "1"),
testRolesAndShares(t, "snowflake_database_grant.test", []string{roleName}, []string{shareName}),
testRolesAndShares(t, "snowflake_database_grant.test", []string{roleName}),
),
},
// IMPORT
Expand Down
3 changes: 1 addition & 2 deletions pkg/resources/failover_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ func ReadFailoverGroup(d *schema.ResourceData, meta interface{}) error {

found := false
for _, fg := range failoverGroups {

if fg.Name.String == name && fg.AccountLocator.String == accountLocator {
found = true
err = d.Set("name", fg.Name.String)
Expand All @@ -300,7 +299,7 @@ func ReadFailoverGroup(d *schema.ResourceData, meta interface{}) error {
}

// this is basically a hack to get around the fact that the API returns the object types in a different order than what is set
// this logic could also be put in the diff supress function, but I think it is better to do it here
// this logic could also be put in the diff suppress function, but I think it is better to do it here.
currentObjectTypeList := d.Get("object_types").(*schema.Set).List()
if len(currentObjectTypeList) != len(objectTypes) {
log.Printf("[DEBUG] object types are different, current: %v, new: %v", currentObjectTypeList, objectTypes)
Expand Down
1 change: 0 additions & 1 deletion pkg/resources/file_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,6 @@ func UpdateFileFormat(d *schema.ResourceData, meta interface{}) error {
if err != nil {
return errors.Wrapf(err, "error updating file format file extension on %v", d.Id())
}

}

if d.HasChange("skip_header") {
Expand Down
24 changes: 12 additions & 12 deletions pkg/resources/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ func prepDummyFunctionResource(t *testing.T) *schema.ResourceData {
argument1 := map[string]interface{}{"name": "data", "type": "varchar"}
argument2 := map[string]interface{}{"name": "event_dt", "type": "date"}
in := map[string]interface{}{
"name": "my_funct",
"database": "my_db",
"schema": "my_schema",
"arguments": []interface{}{argument1, argument2},
"language": "PYTHON",
"null_input_behaviour": "CALLED ON NULL INPUT",
"return_behavior": "VOLATILE",
"runtime_version": "3.8",
"packages": []interface{}{"numpy", "pandas"},
"handler": "add_py",
"return_type": "varchar",
"statement": functionBody, //var message = DATA + DATA;return message
"name": "my_funct",
"database": "my_db",
"schema": "my_schema",
"arguments": []interface{}{argument1, argument2},
"language": "PYTHON",
"null_input_behavior": "CALLED ON NULL INPUT",
"return_behavior": "VOLATILE",
"runtime_version": "3.8",
"packages": []interface{}{"numpy", "pandas"},
"handler": "add_py",
"return_type": "varchar",
"statement": functionBody, //var message = DATA + DATA;return message
}
d := function(t, "my_db|my_schema|my_funct|VARCHAR-DATE", in)
return d
Expand Down
3 changes: 2 additions & 1 deletion pkg/resources/grant_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ func readGenericGrant(
grantSchema map[string]*schema.Schema,
builder snowflake.GrantBuilder,
futureObjects bool,
validPrivileges PrivilegeSet) error {
validPrivileges PrivilegeSet,
) error {
db := meta.(*sql.DB)
var grants []*grant
var err error
Expand Down
1 change: 0 additions & 1 deletion pkg/resources/grant_helpers_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ func TestGrantLegacyID(t *testing.T) {
r.Equal("priv", grant.Privilege)
r.Equal([]string{}, grant.Roles)
r.Equal(false, grant.GrantOption)

}

func TestGrantIDFromStringRoleGrant(t *testing.T) {
Expand Down
34 changes: 17 additions & 17 deletions pkg/resources/notification_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,50 +13,50 @@ import (

var notificationIntegrationSchema = map[string]*schema.Schema{
// The first part of the schema is shared between all integration vendors
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"enabled": &schema.Schema{
"enabled": {
Type: schema.TypeBool,
Optional: true,
Default: true,
},
"type": &schema.Schema{
"type": {
Type: schema.TypeString,
Optional: true,
Default: "QUEUE",
ValidateFunc: validation.StringInSlice([]string{"QUEUE"}, true),
Description: "A type of integration",
ForceNew: true,
},
"direction": &schema.Schema{
"direction": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{"INBOUND", "OUTBOUND"}, true),
Description: "Direction of the cloud messaging with respect to Snowflake (required only for error notifications)",
ForceNew: true,
},
// This part of the schema is the cloudProviderParams in the Snowflake documentation and differs between vendors
"notification_provider": &schema.Schema{
"notification_provider": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{"AZURE_STORAGE_QUEUE", "AWS_SQS", "AWS_SNS", "GCP_PUBSUB"}, true),
Description: "The third-party cloud message queuing service (e.g. AZURE_STORAGE_QUEUE, AWS_SQS, AWS_SNS)",
ForceNew: true,
},
"azure_storage_queue_primary_uri": &schema.Schema{
"azure_storage_queue_primary_uri": {
Type: schema.TypeString,
Optional: true,
Description: "The queue ID for the Azure Queue Storage queue created for Event Grid notifications",
},
"azure_tenant_id": &schema.Schema{
"azure_tenant_id": {
Type: schema.TypeString,
Optional: true,
Description: "The ID of the Azure Active Directory tenant used for identity management",
},
"aws_sqs_external_id": &schema.Schema{
"aws_sqs_external_id": {
Type: schema.TypeString,
Computed: true,
Description: "The external ID that Snowflake will use when assuming the AWS role",
Expand All @@ -66,17 +66,17 @@ var notificationIntegrationSchema = map[string]*schema.Schema{
Computed: true,
Description: "The Snowflake user that will attempt to assume the AWS role.",
},
"aws_sqs_arn": &schema.Schema{
"aws_sqs_arn": {
Type: schema.TypeString,
Optional: true,
Description: "AWS SQS queue ARN for notification integration to connect to",
},
"aws_sqs_role_arn": &schema.Schema{
"aws_sqs_role_arn": {
Type: schema.TypeString,
Optional: true,
Description: "AWS IAM role ARN for notification integration to assume",
},
"aws_sns_external_id": &schema.Schema{
"aws_sns_external_id": {
Type: schema.TypeString,
Computed: true,
Description: "The external ID that Snowflake will use when assuming the AWS role",
Expand All @@ -86,32 +86,32 @@ var notificationIntegrationSchema = map[string]*schema.Schema{
Computed: true,
Description: "The Snowflake user that will attempt to assume the AWS role.",
},
"aws_sns_topic_arn": &schema.Schema{
"aws_sns_topic_arn": {
Type: schema.TypeString,
Optional: true,
Description: "AWS SNS Topic ARN for notification integration to connect to",
},
"aws_sns_role_arn": &schema.Schema{
"aws_sns_role_arn": {
Type: schema.TypeString,
Optional: true,
Description: "AWS IAM role ARN for notification integration to assume",
},
"comment": &schema.Schema{
"comment": {
Type: schema.TypeString,
Optional: true,
Description: "A comment for the integration",
},
"created_on": &schema.Schema{
"created_on": {
Type: schema.TypeString,
Computed: true,
Description: "Date and time when the notification integration was created.",
},
"gcp_pubsub_subscription_name": &schema.Schema{
"gcp_pubsub_subscription_name": {
Type: schema.TypeString,
Optional: true,
Description: "The subscription id that Snowflake will listen to when using the GCP_PUBSUB provider.",
},
"gcp_pubsub_service_account": &schema.Schema{
"gcp_pubsub_service_account": {
Type: schema.TypeString,
Computed: true,
Description: "The GCP service account identifier that Snowflake will use when assuming the GCP role",
Expand Down
14 changes: 8 additions & 6 deletions pkg/resources/notification_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,15 @@ func TestNotificationIntegrationCreate(t *testing.T) {
expectSQL: `^CREATE NOTIFICATION INTEGRATION "test_notification_integration" COMMENT='great comment' GCP_PUBSUB_SUBSCRIPTION_NAME='some-gcp-sub-name' NOTIFICATION_PROVIDER='GCP_PUBSUB' TYPE='QUEUE' ENABLED=true$`,
},
}
for _, testCase := range testCases {
for _, tc := range testCases {
tc := tc
r := require.New(t)
d := schema.TestResourceDataRaw(t, resources.NotificationIntegration().Schema, testCase.raw)
d := schema.TestResourceDataRaw(t, resources.NotificationIntegration().Schema, tc.raw)
r.NotNil(d)

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(testCase.expectSQL).WillReturnResult(sqlmock.NewResult(1, 1))
expectReadNotificationIntegration(mock, testCase.notificationProvider)
mock.ExpectExec(tc.expectSQL).WillReturnResult(sqlmock.NewResult(1, 1))
expectReadNotificationIntegration(mock, tc.notificationProvider)

err := resources.CreateNotificationIntegration(d, db)
r.NoError(err)
Expand All @@ -102,13 +103,14 @@ func TestNotificationIntegrationRead(t *testing.T) {
notificationProvider: "GCP_PUBSUB",
},
}
for _, testCase := range testCases {
for _, tc := range testCases {
tc := tc
r := require.New(t)

d := notificationIntegration(t, "test_notification_integration", map[string]interface{}{"name": "test_notification_integration"})

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
expectReadNotificationIntegration(mock, testCase.notificationProvider)
expectReadNotificationIntegration(mock, tc.notificationProvider)

err := resources.ReadNotificationIntegration(d, db)
r.NoError(err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/resources/pipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ var pipeSchema = map[string]*schema.Schema{
ForceNew: true,
Description: "Specifies the Amazon Resource Name (ARN) for the SNS topic for your S3 bucket.",
},
"integration": &schema.Schema{
"integration": {
Type: schema.TypeString,
Optional: true,
Description: "Specifies an integration for the pipe.",
},
"notification_channel": &schema.Schema{
"notification_channel": {
Type: schema.TypeString,
Computed: true,
Description: "Amazon Resource Name of the Amazon SQS queue for the stage named in the DEFINITION column.",
Expand All @@ -76,7 +76,7 @@ var pipeSchema = map[string]*schema.Schema{
Computed: true,
Description: "Name of the role that owns the pipe.",
},
"error_integration": &schema.Schema{
"error_integration": {
Type: schema.TypeString,
Optional: true,
Description: "Specifies the name of the notification integration used for error notifications.",
Expand Down
5 changes: 3 additions & 2 deletions pkg/resources/pipe_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,11 @@ func TestPipeCopyStatementDiffSuppress(t *testing.T) {
},
}

for name, testCase := range testCases {
for name, tc := range testCases {
tc := tc
t.Run(name, func(t *testing.T) {
r := require.New(t)
r.Equal(testCase.expected, pipeCopyStatementDiffSuppress("", testCase.declared, testCase.showPipes, nil))
r.Equal(tc.expected, pipeCopyStatementDiffSuppress("", tc.declared, tc.showPipes, nil))
})
}
}
Loading

0 comments on commit 792665f

Please sign in to comment.