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: Add service user and legacy service user resources #3119

Merged

Conversation

sfc-gh-asawicki
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki commented Oct 8, 2024

Introduce snowflake_service_user and snowflake_legacy_service_user resources:

  • Add new resources (almost the same schema and the same limitations as snowflake_user resource - read below)
  • Adjust datasource and add type to the describe_output
  • Adjust SDK: handling types
  • Add SDK integration tests to validate and document the Snowflake behavior of handling user types (create, alter set, alter unset, and parameters)
  • Fix TestAcc_User_issue2970 test
  • Add migration guide
  • Generate models and assertions for the new resources
  • Fix sweepers

References: #2951

Implementation considerations

Users of different types differ in Snowflake just slightly - a few fields are restricted from SERVICE and LEGACY_SERVICE type users. We decided to split Snowflake resources into multiple resources on our route to V1, to simplify the logic and usage (in this case, not to handle the type changes which are followed by attributes being hidden/revealed). That led to three choices for the implementation, to reuse the existing resource:

  • copy-paste snowflake_user resource
  • parameterized handling methods of snowflake_user resource with the user type
  • enrich the helper methods with the knowledge of the schema (i.e. to change the logic of setting a field to always check first if the attribute exists in the given schema)

Ultimately, the second option was picked for the current implementation. More details of pros and cons of each solution are presented in points below.

copy-paste

Pros:

  • Fastest implementation
  • We plan to ultimately generate the resources, so at the end the implementation would be close to duplicated resources

Cons:

  • Need to alter the logic in three different places
  • Separate implementation of resources would require separate complete set of tests
parameterized handling methods

Pros:

  • user object handled in "one" place
  • implementation reused, so only smoke acceptance tests are justified

Cons:

  • more complex logic in user resource, spread over multiple handling methods
enrich the helper methods

Pros:

  • user resource implementation could stay the same in helper methods (because the checks would be delegated to helper methods)
  • solution that can be reused in more resources
  • only smoke tests would be justified

Cons:

  • SDKv2 does not allow to fetch the schema in context of helper methods without exposing unexported fields
  • "unsafe" solution potentially used in the majority of resources (because helper methods should be reused more and more)

Copy link

github-actions bot commented Oct 8, 2024

Integration tests failure for c7aca444c4a3e3bae5ce978dd6b5ad38c5150a97

@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review October 8, 2024 20:59
Copy link

github-actions bot commented Oct 8, 2024

Integration tests failure for 7c3737b8ed747eab803c2206d424c475d704d338

MIGRATION_GUIDE.md Show resolved Hide resolved
pkg/resources/custom_diffs.go Outdated Show resolved Hide resolved
templates/resources/service_user.md.tmpl Show resolved Hide resolved
pkg/acceptance/helpers/user_client.go Outdated Show resolved Hide resolved
pkg/resources/user.go Outdated Show resolved Hide resolved
pkg/resources/user.go Show resolved Hide resolved
examples/resources/snowflake_legacy_service_user/import.sh Outdated Show resolved Hide resolved
rsa_public_key_2 = "..."

must_change_password = true
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we create basic and complete version of resource configurations like for other resources?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added for all user types

pkg/acceptance/helpers/user_client.go Outdated Show resolved Hide resolved
pkg/sdk/testint/users_integration_test.go Show resolved Hide resolved
Copy link

github-actions bot commented Oct 9, 2024

Integration tests failure for 3a1ff98589dce8fb8287453f8a746f8963307d58

@sfc-gh-asawicki sfc-gh-asawicki merged commit 0e88e08 into main Oct 10, 2024
8 of 9 checks passed
@sfc-gh-asawicki sfc-gh-asawicki deleted the add-service-user-and-legacy-service-user-resources branch October 10, 2024 08:43
sfc-gh-jcieslak pushed a commit that referenced this pull request Oct 11, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.97.0](v0.96.0...v0.97.0)
(2024-10-10)


### 🎉 **What's new:**

* Add secret to sdk
([#3091](#3091))
([7430aee](7430aee))
* Add service user and legacy service user resources
([#3119](#3119))
([0e88e08](0e88e08))
* Handle all Task parameters in SDK
([#3103](#3103))
([08ae072](08ae072))
* Stream on external table resource
([#3122](#3122))
([d837341](d837341))
* Stream on table resource
([#3109](#3109))
([97fa9b4](97fa9b4))
* Tasks v1 readiness - SDK part
([#3086](#3086))
([0a77383](0a77383))
* Upgrade stream sdk
([#3105](#3105))
([ad5fa11](ad5fa11))


### 🔧 **Misc**

* Add pre check to each datasource
([#3065](#3065))
([560ab6b](560ab6b))
* Bump golang-ci lint to 1.61
([#3112](#3112))
([f23e085](f23e085))
* Secret Validation change
([#3111](#3111))
([666630e](666630e))


### 🐛 **Bug fixes:**

* Fix parsing text in view, check parenthesis in
ParseSchemaObjectIdentifierWithArguments
([#3102](#3102))
([b0a67e6](b0a67e6))
* Try to reproduce 2679 and 3117
([#3124](#3124))
([ccdbc30](ccdbc30))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
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.

3 participants