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

fix: oauth integration #1315

Merged

Conversation

kevinneville
Copy link
Contributor

@kevinneville kevinneville commented Oct 31, 2022

Resolves #785

When using https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/oauth_integration with a custom client we need two required arguments. oauth_redirect_uri and oauth_client type. Today only oauth_redirect_uri is supported.

This PR aims to solve that.

When I use

resource "snowflake_oauth_integration" "new_integration" {
  provider           = snowflake.SYSADMIN
  name               = "NEW_INTEGRATION"
  oauth_client       = "CUSTOM"
  enabled            = true
  oauth_redirect_uri = "MY_URL"
}

I get the following error.

│ Error: error creating security integration: 002029 (42601): SQL compilation error:
│ Missing option(s): OAUTH_CLIENT_TYPE

When using below Terraform code

resource "snowflake_oauth_integration" "new_integration" {
  provider           = snowflake.SYSADMIN
  name               = "NEW_INTEGRATION"
  oauth_client_type  = "CONFIDENTIAL"
  oauth_client       = "CUSTOM"
  enabled            = true
  oauth_redirect_uri = "MY_URL"
}

I get

│ An argument named "oauth_client_type" is not expected here.

References

See https://docs.snowflake.com/en/sql-reference/sql/create-security-integration-oauth-snowflake.html#additional-required-parameters-custom-clients to prove this is needed.

Signed-off-by: Kevin Neville <[email protected]>
@kevinneville
Copy link
Contributor Author

@sfc-gh-swinkler
Hello,
I noticed you worked on the redirect URI, and I tried to follow your steps. Could you review it?
I also think it would be good to add a test for this. But I am unsure how and where to do it, could you assist with this PR?

My gut feeling says somewhere here https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/pkg/provider/provider_test.go#L109 but not sure.

Any advice would be appreciated.

sfc-gh-swinkler
sfc-gh-swinkler previously approved these changes Nov 1, 2022
@sfc-gh-swinkler sfc-gh-swinkler changed the title Fix oauth integration Fix: oauth integration Nov 1, 2022
@kevinneville kevinneville changed the title Fix: oauth integration fix: oauth integration Nov 1, 2022
@kevinneville
Copy link
Contributor Author

@sfc-gh-swinkler Since I am a first-time contributor I can't trigger the builds, would you mind? Then we can merge it.

@sfc-gh-swinkler
Copy link
Collaborator

@kevinneville sorry for the past few days have been having trouble getting the latest release out so was not able to merge PRs. This is failing a couple of minor checks:

  1. go fmt ./... needs to be run
  2. make docs needs to be run

After those both pass i can run unit / integration tests and get this merged. Thanks!

@kevinneville kevinneville force-pushed the fix-oauth-integration branch from 7e55057 to 42eee99 Compare November 2, 2022 10:16
@kevinneville
Copy link
Contributor Author

@kevinneville sorry for the past few days have been having trouble getting the latest release out so was not able to merge PRs. This is failing a couple of minor checks:

  1. go fmt ./... needs to be run
  2. make docs needs to be run

After those both pass i can run unit / integration tests and get this merged. Thanks!

Alright, let me know if I can assist in any way.
I updated the code and those checks should pass now.

@sfc-gh-swinkler
Copy link
Collaborator

/ok-to-test sha=42eee99

@github-actions
Copy link

github-actions bot commented Nov 3, 2022

Integration tests failure for 42eee99

@kevinneville
Copy link
Contributor Author

kevinneville commented Nov 3, 2022

/ok-to-test sha=e58b8fc, almost :D

@sfc-gh-swinkler
Copy link
Collaborator

/ok-to-test sha=e58b8fc

@github-actions
Copy link

github-actions bot commented Nov 7, 2022

Integration tests failure for e58b8fc

@sfc-gh-swinkler
Copy link
Collaborator

== CONT  TestAcc_OAuthIntegration
    oauth_integration_acceptance_test.go:16: Step 1/2 error: Error running apply: exit status 1
        
        Error: error creating security integration: 001420 (22023): SQL compilation error:
        invalid property 'OAUTH_CLIENT_TYPE' for 'INTEGRATION - OAUTH - TABLEAU_SERVER'
        
          on terraform_plugin_test.tf line 3, in resource "snowflake_oauth_integration" "test":
           3: 	resource "snowflake_oauth_integration" "test" {
        
        
--- FAIL: TestAcc_OAuthIntegration (2.15s)

@kevinneville so the value being returned by if err = d.Set("oauth_client_type", v.(string)); err != nil { is not what you think it is. It looks like the value is 'INTEGRATION - OAUTH - TABLEAU_SERVER'. I am not familiar with this. Can "confidential" or "public" be inferred from this value or does it need to be queried from some other API endpoint?

@culpgrant
Copy link

culpgrant commented Jan 26, 2023

Do we think there would be any possibility this could get merged in? I see there is still one remaining issue.
I am trying to set up DBT Cloud and this is required but currently, the resource doesn't work because of the issue in this PR :)

@sfc-gh-swinkler sfc-gh-swinkler merged commit 9087220 into Snowflake-Labs:main Mar 16, 2023
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.

Support Custom OAuth Integration Type
3 participants