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.

Sign up for GitHub

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

Jump to bottom

feat: Add SCIM and SAML2 security integrations to sdk #2799

Merged
merged 18 commits into from
May 20, 2024
Merged

feat: Add SCIM and SAML2 security integrations to sdk #2799

merged 18 commits into from
May 20, 2024

Conversation

sfc-gh-jmichalak
Copy link
Collaborator

@sfc-gh-jmichalak sfc-gh-jmichalak commented May 14, 2024 •
edited
Loading

Add security integrations to SDK.
Changes in follow ups: Add remaining integration types. Use SDK in Terraform resources. Make sure related bugs are fixed.

Test Plan

  • acceptance tests
  • …

References

https://docs.snowflake.com/en/sql-reference/sql/create-security-integration
https://docs.snowflake.com/en/sql-reference/sql/alter-security-integration

@github-actions GitHub Actions
Copy link

Integration tests failure for 0da7ceefdfb1ecd1d50dfd736f0522c3cd622b2f

@sfc-gh-jmichalak sfc-gh-jmichalak changed the title feat: Improve security integrations feat: Add SCIM and SAML2 security integrations to sdk May 14, 2024
@sfc-gh-jmichalak sfc-gh-jmichalak requested review from sfc-gh-jcieslak and sfc-gh-asawicki and removed request for sfc-gh-jcieslak May 14, 2024 10:41
@sfc-gh-jmichalak sfc-gh-jmichalak marked this pull request as ready for review May 14, 2024 10:41
@github-actions GitHub Actions
Copy link

Integration tests failure for fe257780e2b0ead3c097e88685a9b88af72b84e4

@github-actions GitHub Actions
Copy link

Integration tests failure for 68c5b993740fe1ff6ad0e2efdd499eb35e6fd613

@github-actions GitHub Actions
Copy link

Integration tests failure for 0a0f76d31bf2be55721f8208f7cc98b333465bda

@github-actions GitHub Actions
Copy link

Integration tests failure for 0fa4bc15bdc4feff4230f2a0281eba3cd1a22f32

1 similar comment
@github-actions GitHub Actions
Copy link

Integration tests failure for 0fa4bc15bdc4feff4230f2a0281eba3cd1a22f32

sfc-gh-jcieslak
sfc-gh-jcieslak requested changes May 15, 2024
View reviewed changes
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
sfc-gh-asawicki
sfc-gh-asawicki requested changes May 15, 2024
View reviewed changes
pkg/sdk/security_integrations_def.go Outdated Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Outdated Show resolved Hide resolved
pkg/sdk/security_integrations_def.go Show resolved Hide resolved
pkg/sdk/security_integrations_gen_test.go Show resolved Hide resolved
pkg/sdk/security_integrations_gen_test.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Outdated

createSCIMIntegration := func(t *testing.T, siID sdk.AccountObjectIdentifier, with func(*sdk.CreateSCIMSecurityIntegrationRequest)) {
t.Helper()
roleID := sdk.NewAccountObjectIdentifier("GENERIC_SCIM_PROVISIONER")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, we use random names. Use CreateRole from helpers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup, we need roles with names specified in https://docs.snowflake.com/en/sql-reference/sql/create-security-integration-scim (RUN_AS_ROLE)

Copy link
Collaborator

Choose a reason for hiding this comment

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

MMM, ok then, they should be exposed as predefined roles You can add them here now: https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/pkg/internal/snowflakeroles/snowflake_predefined_roles.go

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

pkg/sdk/testint/security_integrations_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for 939f823afd1e18429a2497cf4d7d4e94e0b45ec7

@github-actions GitHub Actions
Copy link

Integration tests failure for 5053b8220b2e1120a7e57d6f9c480a87c3541ade

@sfc-gh-jmichalak sfc-gh-jmichalak force-pushed the jmichalak-security-integrations branch from 5053b82 to 3cc3724 Compare May 16, 2024 09:44
@sfc-gh-jmichalak sfc-gh-jmichalak force-pushed the jmichalak-security-integrations branch from 3cc3724 to 994bcd1 Compare May 16, 2024 09:47
@github-actions GitHub Actions
Copy link

Integration tests failure for 994bcd142f656fe60323ee0bfd9ed10f5065e666

@github-actions GitHub Actions
Copy link

Integration tests failure for 3cc372413fcdd628dd068cf04a52caa98cd5e25b

@github-actions GitHub Actions
Copy link

Integration tests success for 40dde78876ec3aa5d103a0e49270c83c966f9edd

@github-actions GitHub Actions
Copy link

Integration tests failure for b8bc79f19d2f8d77f3343a893f32cbed9f7e1df6

@github-actions GitHub Actions
Copy link

Integration tests failure for 19022afe272c957551edd6fd009cb727d8e4a905

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review May 20, 2024 09:23
@github-actions GitHub Actions
Copy link

Integration tests failure for f9c9d3ca33284c53717348b502eaff5d96e8ca20

sfc-gh-jcieslak
sfc-gh-jcieslak reviewed May 20, 2024
View reviewed changes
pkg/acceptance/helpers/role_client.go Outdated Show resolved Hide resolved
pkg/sdk/security_integrations_gen.go Outdated Show resolved Hide resolved
pkg/sdk/testint/security_integrations_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/acceptance/helpers/security_integration_client.go Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for c5eede3d487c8fa080f45f47a27db7eefdf64c4b

@github-actions GitHub Actions
Copy link

Integration tests success for aa792e2f190f2c874551e8a679acfe1b17654282

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review May 20, 2024 13:27
sfc-gh-asawicki
sfc-gh-asawicki approved these changes May 20, 2024
View reviewed changes
pkg/internal/snowflakeroles/snowflake_predefined_roles.go
Accountadmin = sdk.NewAccountObjectIdentifier("ACCOUNTADMIN")
Orgadmin = sdk.NewAccountObjectIdentifier("ORGADMIN")
Accountadmin = sdk.NewAccountObjectIdentifier("ACCOUNTADMIN")
GenericScimProvisioner = sdk.NewAccountObjectIdentifier("GENERIC_SCIM_PROVISIONER")
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not add also others (there were three in the docs, right?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm, I didn't need this atm, as the remaining roles are present in defs file. I can add them here in follow ups tho

@sfc-gh-jmichalak sfc-gh-jmichalak merged commit 1312ff1 into main May 20, 2024
9 checks passed
@sfc-gh-jmichalak sfc-gh-jmichalak deleted the jmichalak-security-integrations branch May 20, 2024 13:38
@snowflake-release-please snowflake-release-please bot mentioned this pull request May 20, 2024
Merged
sfc-gh-jcieslak pushed a commit that referenced this pull request Jun 6, 2024
@snowflake-release-please
chore(main): release 0.92.0 (#2824)
5df6d3d 
🤖 I have created a release *beep* *boop*
---


##
[0.92.0](v0.91.0...v0.92.0)
(2024-06-06)


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

* Add Api Authentication security integration to sdk
([#2840](#2840))
([57a07ee](57a07ee))
* Add External Oauth security integration to sdk
([#2835](#2835))
([82d1c09](82d1c09))
* add network rules
([#2746](#2746))
([c79fa29](c79fa29))
* Add SCIM and SAML2 security integrations to sdk
([#2799](#2799))
([1312ff1](1312ff1))
* Add Snowflake Oauth security integration to sdk
([#2830](#2830))
([b576f29](b576f29))
* Database resource v1 readiness
([#2834](#2834))
([30fe136](30fe136))
* Database SDK upgrade
([#2814](#2814))
([750fe37](750fe37))


### 🔧 **Misc**

* accept non-pointer values in the generated builder methods
([#2816](#2816))
([c29fbf1](c29fbf1))
* Add a script for creating labels
([#2778](#2778))
([ce0fbad](ce0fbad))
* Adjust before 0.92.0
([#2857](#2857))
([0598656](0598656))
* Continue random ids rework
([#2819](#2819))
([f20940c](f20940c))
* Random ids rework part3
([#2833](#2833))
([36ead85](36ead85))
* Random ids rework part4
([#2837](#2837))
([64518a3](64518a3))
* Update issue for table and warehouse redesign state
([#2845](#2845))
([149e55e](149e55e))


### 🐛 **Bug fixes:**

* Fix failing integration tests
([#2832](#2832))
([2e2ca6c](2e2ca6c))
* Fix QUOTED_IDENTIFIERS_IGNORE_CASE parameter test
([#2841](#2841))
([92ad1d3](92ad1d3))

---
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
Reviewers

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak approved these changes

@sfc-gh-asawicki sfc-gh-asawicki sfc-gh-asawicki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-jmichalak @sfc-gh-jcieslak @sfc-gh-asawicki