-
Notifications
You must be signed in to change notification settings - Fork 418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Secret resource #3110
feat: Secret resource #3110
Conversation
…e and updated MIGRATION_GUIDE.md
pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/secret_show_output_gen.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_oauth_client_credentials_acceptance_test.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_oauth_client_credentials_acceptance_test.go
Outdated
Show resolved
Hide resolved
Integration tests failure for 9dea35cc44894aa14b7aee86e70ff75f166591b3 |
pkg/resources/secret_with_basic_authentication_acceptance_test.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_basic_authentication_acceptance_test.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_oauth_client_credentials_acceptance_test.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_oauth_authorization_code_grant_acceptance_test.go
Outdated
Show resolved
Hide resolved
)) | ||
}, | ||
Config: config.FromModel(t, secretModel), | ||
Config: config.FromModel(t, secretModelExternalChanges), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not quite sure about this test, why we alter both the config and externally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because i didnt correct this one after changes to handling token_expiry_time
field
I will correct that and use properly, it should test the external change
pkg/resources/secret_with_oauth_authorization_code_grant_acceptance_test.go
Outdated
Show resolved
Hide resolved
pkg/resources/secret_with_oauth_client_credentials_acceptance_test.go
Outdated
Show resolved
Hide resolved
…oint from 0.97 -> 0.98
Integration tests failure for 1a9a6db5d90a1aae584ad610aa61662257321624 |
|
||
//go:generate go run ./poc/main.go | ||
const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we usually have separated types for such groups of enum values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be added with the PR for externally changed secret_types
Integration tests failure for b202fdde49d84844251674844f50696e17315af8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update on main to have a longer timeout for acceptance tests
@@ -67,8 +67,9 @@ func TestAcc_SecretWithBasicAuthentication_BasicFlow(t *testing.T) { | |||
resource.TestCheckResourceAttr(secretName, "describe_output.0.name", name), | |||
resource.TestCheckResourceAttr(secretName, "describe_output.0.database_name", id.DatabaseName()), | |||
resource.TestCheckResourceAttr(secretName, "describe_output.0.schema_name", id.SchemaName()), | |||
resource.TestCheckResourceAttr(secretName, "describe_output.0.secret_type", "PASSWORD"), | |||
resource.TestCheckResourceAttr(secretName, "describe_output.0.secret_type", sdk.SecretTypePassword), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the separate type change you will have to do sdk.SecretTypePassword -> string(sdk.SecretTypePassword) in multiple tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be changed in a PR with the secret type change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, I will keep this open then
@@ -43,6 +43,28 @@ type showMapping struct { | |||
normalizeFunc func(any) any | |||
} | |||
|
|||
// handleExternalValueChangesToObjectInDescribe assumes that describe output is kept in DescribeOutputAttributeName attribute | |||
func handleExternalValueChangesToObjectInDescribe(d *schema.ResourceData, mappings ...describeMapping) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is copy-paste of handleExternalChangesToObjectInShow, only the var names and attribute name changes. Please extract this as a common body (it may be done in a separate PR)
Integration tests failure for b202fdde49d84844251674844f50696e17315af8 |
Changes
snowflake_secret_with_client_credentials
resourcesnowflake_secret_with_authorization_code_grant
resourcesnowflake_secret_with_basic_authentication
resourcesnowflake_secret_with_generic_string
resourceParseCommaSeparatedStringArray()
Test Plan
References
https://docs.snowflake.com/en/sql-reference/sql/create-secret
TODO