Skip to content
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

Merged
merged 73 commits into from
Oct 17, 2024
Merged

feat: Secret resource #3110

merged 73 commits into from
Oct 17, 2024

Conversation

sfc-gh-fbudzynski
Copy link
Collaborator

@sfc-gh-fbudzynski sfc-gh-fbudzynski commented Oct 2, 2024

Changes

  • add snowflake_secret_with_client_credentials resource
  • add snowflake_secret_with_authorization_code_grant resource
  • add snowflake_secret_with_basic_authentication resource
  • add snowflake_secret_with_generic_string resource
  • fix parsing oauth_scopes list with ParseCommaSeparatedStringArray()

Test Plan

  • acceptance tests

References

https://docs.snowflake.com/en/sql-reference/sql/create-secret

TODO

  • datasource
  • tests for externally changed secret type

Copy link

Integration tests failure for 9dea35cc44894aa14b7aee86e70ff75f166591b3

))
},
Config: config.FromModel(t, secretModel),
Config: config.FromModel(t, secretModelExternalChanges),
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

docs/resources/secret_with_authorization_code_grant.md Outdated Show resolved Hide resolved
pkg/resources/secret_common.go Outdated Show resolved Hide resolved
pkg/resources/secret_common.go Outdated Show resolved Hide resolved
pkg/resources/secret_common.go Outdated Show resolved Hide resolved
pkg/resources/secret_with_basic_authentication.go Outdated Show resolved Hide resolved
Copy link

Integration tests failure for 1a9a6db5d90a1aae584ad610aa61662257321624


//go:generate go run ./poc/main.go
const (
Copy link
Collaborator

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.

Copy link
Collaborator Author

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

Copy link

Integration tests failure for b202fdde49d84844251674844f50696e17315af8

Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki left a 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),
Copy link
Collaborator

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

Copy link
Collaborator Author

@sfc-gh-fbudzynski sfc-gh-fbudzynski Oct 16, 2024

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

Copy link
Collaborator

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 {
Copy link
Collaborator

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)

Copy link

Integration tests failure for b202fdde49d84844251674844f50696e17315af8

@sfc-gh-fbudzynski sfc-gh-fbudzynski merged commit 16a812d into main Oct 17, 2024
8 of 9 checks passed
@sfc-gh-fbudzynski sfc-gh-fbudzynski deleted the secret-resource branch October 17, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants