Skip to content

Commit

Permalink
chore: Secret Validation change (#3111)
Browse files Browse the repository at this point in the history 
## Changes
- changed secret validation to use generated validation without manual
adjustments
- changed parsing StringArrays to use `ParseCommaSeparatedStringArray()`
- removed known issue with ConflictingFields from sdk generator README
- removed comments and replaced them with jira issues
  • Loading branch information
@sfc-gh-fbudzynski
sfc-gh-fbudzynski authored Oct 4, 2024
1 parent b0a67e6 commit 666630e
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 135 deletions.
23 changes: 0 additions & 23 deletions pkg/sdk/poc/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,29 +132,6 @@ type SomeReq struct {
}
```

- Wrong generated validations for `ConflictingFields` for more complicated validations
- For now `everyValueSet()` should be manually changed to `moreThanOneValueSet()` if there is a need to validate only one of many options
- Consider following example for `secret`:
```go
// For validation Conflicting fields:
WithValidation(g.ConflictingFields, "SetForOAuthClientCredentialsFlow", "SetForOAuthAuthorizationFlow", "SetForBasicAuthentication", "SetForGenericString")
```
```go
// Generation results in:
if valueSet(opts.Set) {
if everyValueSet(opts.Set.SetForOAuthClientCredentialsFlow, opts.Set.SetForOAuthAuthorizationFlow, opts.Set.SetForBasicAuthentication, opts.Set.SetForGenericString) {
errs = append(errs, errOneOf("AlterSecretOptions.Set", "SetForOAuthClientCredentialsFlow", "SetForOAuthAuthorizationFlow", "SetForBasicAuthentication", "SetForGenericString"))
}
}

// For now needs to be manually changed to:
if valueSet(opts.Set) {
if moreThanOneValueSet(opts.Set.SetForOAuthClientCredentialsFlow, opts.Set.SetForOAuthAuthorizationFlow, opts.Set.SetForBasicAuthentication, opts.Set.SetForGenericString) {
errs = append(errs, errOneOf("AlterSecretOptions.Set", "SetForOAuthClientCredentialsFlow", "SetForOAuthAuthorizationFlow", "SetForBasicAuthentication", "SetForGenericString"))
}
}
```

##### Known limitations
- automatic array conversion is not recursive, so we're only supporting one level mapping
- []Request1{ foo Request2, bar int } won't be converted, but []Request1{ foo string, bar int } will
56 changes: 31 additions & 25 deletions pkg/sdk/secrets_def.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,34 +62,40 @@ var oauthScopesListDef = g.NewQueryStruct("OauthScopesList").List("OauthScopesLi
var secretSet = g.NewQueryStruct("SecretSet").
OptionalComment().
OptionalQueryStructField(
"SetForOAuthClientCredentialsFlow",
g.NewQueryStruct("SetForOAuthClientCredentialsFlow").
OptionalQueryStructField("OauthScopes", oauthScopesListDef, g.ParameterOptions().SQL("OAUTH_SCOPES").Parentheses()),
"SetForFlow",
g.NewQueryStruct("SetForFlow").
OptionalQueryStructField(
"SetForOAuthClientCredentials",
g.NewQueryStruct("SetForOAuthClientCredentials").
OptionalQueryStructField("OauthScopes", oauthScopesListDef, g.ParameterOptions().SQL("OAUTH_SCOPES").Parentheses()),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForOAuthAuthorization",
g.NewQueryStruct("SetForOAuthAuthorization").
OptionalTextAssignment("OAUTH_REFRESH_TOKEN", g.ParameterOptions().SingleQuotes()).
OptionalTextAssignment("OAUTH_REFRESH_TOKEN_EXPIRY_TIME", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForBasicAuthentication",
g.NewQueryStruct("SetForBasicAuthentication").
OptionalTextAssignment("USERNAME", g.ParameterOptions().SingleQuotes()).
OptionalTextAssignment("PASSWORD", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForGenericString",
g.NewQueryStruct("SetForGenericString").
OptionalTextAssignment("SECRET_STRING", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
WithValidation(g.ExactlyOneValueSet, "SetForOAuthClientCredentials", "SetForOAuthAuthorization", "SetForBasicAuthentication", "SetForGenericString"),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForOAuthAuthorizationFlow",
g.NewQueryStruct("SetForOAuthAuthorizationFlow").
OptionalTextAssignment("OAUTH_REFRESH_TOKEN", g.ParameterOptions().SingleQuotes()).
OptionalTextAssignment("OAUTH_REFRESH_TOKEN_EXPIRY_TIME", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForBasicAuthentication",
g.NewQueryStruct("SetForBasicAuthentication").
OptionalTextAssignment("USERNAME", g.ParameterOptions().SingleQuotes()).
OptionalTextAssignment("PASSWORD", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
OptionalQueryStructField(
"SetForGenericString",
g.NewQueryStruct("SetForGenericString").
OptionalTextAssignment("SECRET_STRING", g.ParameterOptions().SingleQuotes()),
g.KeywordOptions(),
).
WithValidation(g.ConflictingFields, "SetForOAuthClientCredentialsFlow", "SetForOAuthAuthorizationFlow", "SetForBasicAuthentication", "SetForGenericString")
WithValidation(g.AtLeastOneValueSet, "SetForFlow", "Comment")

// UNSET doest work, need to use "SET COMMENT = NULL"
// TODO [SNOW-1678749]: Change to use UNSET when it will be possible
var secretUnset = g.NewQueryStruct("SecretUnset").
PredefinedQueryStructField("Comment", "*bool", g.KeywordOptions().SQL("SET COMMENT = NULL"))

Expand Down
35 changes: 22 additions & 13 deletions pkg/sdk/secrets_dto_builders_gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 11 additions & 7 deletions pkg/sdk/secrets_dto_gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,22 @@ type AlterSecretRequest struct {
}

type SecretSetRequest struct {
Comment *string
SetForOAuthClientCredentialsFlow *SetForOAuthClientCredentialsFlowRequest
SetForOAuthAuthorizationFlow *SetForOAuthAuthorizationFlowRequest
SetForBasicAuthentication *SetForBasicAuthenticationRequest
SetForGenericString *SetForGenericStringRequest
Comment *string
SetForFlow *SetForFlowRequest
}

type SetForOAuthClientCredentialsFlowRequest struct {
type SetForFlowRequest struct {
SetForOAuthClientCredentials *SetForOAuthClientCredentialsRequest
SetForOAuthAuthorization *SetForOAuthAuthorizationRequest
SetForBasicAuthentication *SetForBasicAuthenticationRequest
SetForGenericString *SetForGenericStringRequest
}

type SetForOAuthClientCredentialsRequest struct {
OauthScopes *OauthScopesListRequest
}

type SetForOAuthAuthorizationFlowRequest struct {
type SetForOAuthAuthorizationRequest struct {
OauthRefreshToken *string
OauthRefreshTokenExpiryTime *string
}
Expand Down
17 changes: 10 additions & 7 deletions pkg/sdk/secrets_gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,19 @@ type AlterSecretOptions struct {
Unset *SecretUnset `ddl:"keyword"`
}
type SecretSet struct {
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
SetForOAuthClientCredentialsFlow *SetForOAuthClientCredentialsFlow `ddl:"keyword"`
SetForOAuthAuthorizationFlow *SetForOAuthAuthorizationFlow `ddl:"keyword"`
SetForBasicAuthentication *SetForBasicAuthentication `ddl:"keyword"`
SetForGenericString *SetForGenericString `ddl:"keyword"`
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
SetForFlow *SetForFlow `ddl:"keyword"`
}
type SetForOAuthClientCredentialsFlow struct {
type SetForFlow struct {
SetForOAuthClientCredentials *SetForOAuthClientCredentials `ddl:"keyword"`
SetForOAuthAuthorization *SetForOAuthAuthorization `ddl:"keyword"`
SetForBasicAuthentication *SetForBasicAuthentication `ddl:"keyword"`
SetForGenericString *SetForGenericString `ddl:"keyword"`
}
type SetForOAuthClientCredentials struct {
OauthScopes *OauthScopesList `ddl:"parameter,parentheses" sql:"OAUTH_SCOPES"`
}
type SetForOAuthAuthorizationFlow struct {
type SetForOAuthAuthorization struct {
OauthRefreshToken *string `ddl:"parameter,single_quotes" sql:"OAUTH_REFRESH_TOKEN"`
OauthRefreshTokenExpiryTime *string `ddl:"parameter,single_quotes" sql:"OAUTH_REFRESH_TOKEN_EXPIRY_TIME"`
}
Expand Down
46 changes: 33 additions & 13 deletions pkg/sdk/secrets_gen_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,26 +196,46 @@ func TestSecrets_Alter(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterSecretOptions", "Set", "Unset"))
})

t.Run("validation: conflicting fields for [opts.Set.SetForOAuthClientCredentialsFlow opts.Set.SetForOAuthAuthorizationFlow opts.Set.SetForBasicAuthentication opts.Set.SetForGenericString]", func(t *testing.T) {
t.Run("validation: conflicting fields for [opts.Set.SetForFlow.SetForOAuthClientCredentials opts.Set.SetForFlow.SetForOAuthAuthorization opts.Set.SetForFlow.SetForBasicAuthentication opts.Set.SetForFlow.SetForGenericString]", func(t *testing.T) {
opts := setOpts()
opts.Set.SetForOAuthClientCredentialsFlow = &SetForOAuthClientCredentialsFlow{OauthScopes: &OauthScopesList{[]ApiIntegrationScope{{Scope: "foo"}}}}
opts.Set.SetForOAuthAuthorizationFlow = &SetForOAuthAuthorizationFlow{OauthRefreshToken: String("foo"), OauthRefreshTokenExpiryTime: String("bar")}
opts.Set.SetForBasicAuthentication = &SetForBasicAuthentication{Username: String("foo"), Password: String("bar")}
opts.Set.SetForGenericString = &SetForGenericString{SecretString: String("secret")}
assertOptsInvalidJoinedErrors(t, opts, errOneOf("AlterSecretOptions.Set", "SetForOAuthClientCredentialsFlow", "SetForOAuthAuthorizationFlow", "SetForBasicAuthentication", "SetForGenericString"))
opts.Set.SetForFlow = &SetForFlow{}
opts.Set.SetForFlow.SetForOAuthClientCredentials = &SetForOAuthClientCredentials{OauthScopes: &OauthScopesList{[]ApiIntegrationScope{{Scope: "foo"}}}}
opts.Set.SetForFlow.SetForOAuthAuthorization = &SetForOAuthAuthorization{OauthRefreshToken: String("foo"), OauthRefreshTokenExpiryTime: String("bar")}
opts.Set.SetForFlow.SetForBasicAuthentication = &SetForBasicAuthentication{Username: String("foo"), Password: String("bar")}
opts.Set.SetForFlow.SetForGenericString = &SetForGenericString{SecretString: String("secret")}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterSecretOptions.Set.SetForFlow", "SetForOAuthClientCredentials", "SetForOAuthAuthorization", "SetForBasicAuthentication", "SetForGenericString"))
})

t.Run("validation: not every fields specified for conflicting fields [opts.Set.SetForFlow.SetForOAuthClientCredentials opts.Set.SetForFlow.SetForOAuthAuthorization opts.Set.SetForFlow.SetForBasicAuthentication opts.Set.SetForFlow.SetForGenericString]", func(t *testing.T) {
opts := setOpts()
opts.Set.SetForFlow = &SetForFlow{}
opts.Set.SetForFlow.SetForBasicAuthentication = &SetForBasicAuthentication{Username: String("foo"), Password: String("bar")}
opts.Set.SetForFlow.SetForGenericString = &SetForGenericString{SecretString: String("secret")}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterSecretOptions.Set.SetForFlow", "SetForOAuthClientCredentials", "SetForOAuthAuthorization", "SetForBasicAuthentication", "SetForGenericString"))
})

t.Run("validation: valid comment only for at lease one of [opts.Set.SetForFlow opts.Set.Comment]", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
assertOptsValid(t, opts)
})

t.Run("validation: at least one of [opts.Set.SetForFlow opts.Set.Comment]", func(t *testing.T) {
opts := setOpts()
assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterSecretOptions.Set", "SetForFlow", "Comment"))
})

t.Run("alter: set options for Oauth Client Credentials Flow", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
opts.Set.SetForOAuthClientCredentialsFlow = &SetForOAuthClientCredentialsFlow{OauthScopes: &OauthScopesList{[]ApiIntegrationScope{{"sample_scope"}}}}
opts.Set.SetForFlow = &SetForFlow{SetForOAuthClientCredentials: &SetForOAuthClientCredentials{OauthScopes: &OauthScopesList{[]ApiIntegrationScope{{"sample_scope"}}}}}
assertOptsValidAndSQLEquals(t, opts, "ALTER SECRET IF EXISTS %s SET COMMENT = 'test' OAUTH_SCOPES = ('sample_scope')", id.FullyQualifiedName())
})

t.Run("alter: set options for Oauth Client Credentials Flow - empty oauth scopes list", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
opts.Set.SetForOAuthClientCredentialsFlow = &SetForOAuthClientCredentialsFlow{OauthScopes: &OauthScopesList{}}
opts.Set.SetForFlow = &SetForFlow{SetForOAuthClientCredentials: &SetForOAuthClientCredentials{OauthScopes: &OauthScopesList{}}}
assertOptsValidAndSQLEquals(t, opts, "ALTER SECRET IF EXISTS %s SET COMMENT = 'test' OAUTH_SCOPES = ()", id.FullyQualifiedName())
})

Expand All @@ -228,20 +248,20 @@ func TestSecrets_Alter(t *testing.T) {
t.Run("alter: set options for Oauth Authorization Flow", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
opts.Set.SetForOAuthAuthorizationFlow = &SetForOAuthAuthorizationFlow{
opts.Set.SetForFlow = &SetForFlow{SetForOAuthAuthorization: &SetForOAuthAuthorization{
OauthRefreshToken: String("test_token"),
OauthRefreshTokenExpiryTime: String("2024-11-11"),
}
}}
assertOptsValidAndSQLEquals(t, opts, "ALTER SECRET IF EXISTS %s SET COMMENT = 'test' OAUTH_REFRESH_TOKEN = 'test_token' OAUTH_REFRESH_TOKEN_EXPIRY_TIME = '2024-11-11'", id.FullyQualifiedName())
})

t.Run("alter: set options for Basic Authentication", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
opts.Set.SetForBasicAuthentication = &SetForBasicAuthentication{
opts.Set.SetForFlow = &SetForFlow{SetForBasicAuthentication: &SetForBasicAuthentication{
Username: String("foo"),
Password: String("bar"),
}
}}
assertOptsValidAndSQLEquals(t, opts, "ALTER SECRET IF EXISTS %s SET COMMENT = 'test' USERNAME = 'foo' PASSWORD = 'bar'", id.FullyQualifiedName())
})

Expand All @@ -254,7 +274,7 @@ func TestSecrets_Alter(t *testing.T) {
t.Run("alter: set options for Generic string", func(t *testing.T) {
opts := setOpts()
opts.Set.Comment = String("test")
opts.Set.SetForGenericString = &SetForGenericString{String("test")}
opts.Set.SetForFlow = &SetForFlow{SetForGenericString: &SetForGenericString{String("test")}}
assertOptsValidAndSQLEquals(t, opts, "ALTER SECRET IF EXISTS %s SET COMMENT = 'test' SECRET_STRING = 'test'", id.FullyQualifiedName())
})

Expand Down
Loading

0 comments on commit 666630e

Please sign in to comment.