Skip to content

Commit 941c918

Browse files
fix: Fix using lowercased network policies in security integrations (#3867)
Fix setting network policies with lowercase characters in security integrations - Modify the SDK to properly quote network policy names in security integrations - Added integration tests to verify the fix works - Remove the warning note from the documentation - Updated the migration guide with information about the fix - This fix contains a workaround, as the SDK doesn't support custom quoting in identifiers nicely. Added a note to the generator readme. - Adjust the def file with the required `oauth_redirect_uri`. The generated code was already good, but only the def was updated. Fix the example docs as well. - Move some manually added code to an ext file. ## References - #3229 - SNOW-1833593 - https://docs.snowflake.com/en/sql-reference/sql/create-security-integration-oauth-snowflake
1 parent df8c26c commit 941c918

16 files changed

+210
-156
lines changed

MIGRATION_GUIDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ Previously, this field was read-only. In this version, this field is an optional
2929

3030
Note that this resource is still in preview, and not officially supported. This change was requested and done by the community: [#3659](https://github.com/snowflakedb/terraform-provider-snowflake/pull/3659).
3131

32+
### *(bugfix)* Fix setting network policies with lowercase characters in security integrations
33+
Previously, when the provider created or set a security integration (in `snowflake_oauth_integration_for_custom_clients` or `snowflake_scim_integration`) with a network policy containing lowercase letters, this could fail due to a different quoting used in Snowflake in these objects. Namely, despite using the `"` quotes, the referenced network name was uppercased in Snowflake. This means that the uppercased network policy was used instead.
34+
Snowflake could return errors like `Network policy TEST does not exist or not authorized.`.
35+
In this case, a special quoting needs to be used (see [docs](https://docs.snowflake.com/en/sql-reference/sql/create-security-integration-oauth-snowflake)). Instead of the usual `NETWORK_POLICY = "test"`, it needs to be `NETWORK_POLICY = '"test"'`.
36+
37+
In this version, this behavior is fixed. The provider always uses the mixed `'"name"'` notation, and the casing should match the name in Snowflake.
38+
39+
References: [#3229](https://github.com/snowflakedb/terraform-provider-snowflake/issues/3229)
40+
3241
## v2.3.0 ➞ v2.4.0
3342

3443
### *(new feature)* snowflake_current_organization_account resource

docs/resources/oauth_integration_for_custom_clients.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ description: |-
77

88
!> **Sensitive values** This resource's `oauth_redirect_uri` and `describe_output.oauth_redirect_uri` fields are not marked as sensitive in the provider. Ensure that no personal data, sensitive data, export-controlled data, or other regulated data is entered as metadata when using the provider. If you use one of these fields, they may be present in logs, so ensure that the provider logs are properly restricted. For more information, see [Sensitive values limitations](../#sensitive-values-limitations) and [Metadata fields in Snowflake](https://docs.snowflake.com/en/sql-reference/metadata).
99

10-
!> **Note** Setting a network policy with lowercase letters does not work correctly in Snowflake (see [issue](https://github.com/snowflakedb/terraform-provider-snowflake/issues/3229)). As a workaround, set the network policy with uppercase letters only, or use [execute](./execute) with network policy ID wrapped in `'`.
11-
1210
!> **Note** The provider does not detect external changes on security integration type. In this case, remove the integration of wrong type manually with `terraform destroy` and recreate the resource. It will be addressed in the future.
1311

1412
~> **Missing fields** The `oauth_client_id` and `oauth_redirect_uri` fields are not present in the `describe_output` on purpose due to Terraform SDK limitations (more on that in the [migration guide](https://github.com/snowflakedb/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#removal-of-sensitive-fields)).

docs/resources/oauth_integration_for_partner_applications.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ resource "snowflake_oauth_integration_for_partner_applications" "test" {
3333
enabled = "true"
3434
oauth_issue_refresh_tokens = "true"
3535
oauth_refresh_token_validity = 3600
36+
oauth_redirect_uri = "http://example.com"
3637
oauth_use_secondary_roles = "IMPLICIT"
3738
blocked_roles_list = ["ACCOUNTADMIN", "SECURITYADMIN", snowflake_role.one.fully_qualified_name, snowflake_role.two.fully_qualified_name]
3839
comment = "example oauth integration for partner applications"

examples/resources/snowflake_oauth_integration_for_partner_applications/resource.tf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ resource "snowflake_oauth_integration_for_partner_applications" "test" {
1212
enabled = "true"
1313
oauth_issue_refresh_tokens = "true"
1414
oauth_refresh_token_validity = 3600
15+
oauth_redirect_uri = "http://example.com"
1516
oauth_use_secondary_roles = "IMPLICIT"
1617
blocked_roles_list = ["ACCOUNTADMIN", "SECURITYADMIN", snowflake_role.one.fully_qualified_name, snowflake_role.two.fully_qualified_name]
1718
comment = "example oauth integration for partner applications"

pkg/acceptance/helpers/network_rule_client.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@ func (c *NetworkRuleClient) CreateIngress(t *testing.T) (*sdk.NetworkRule, func(
3939
))
4040
}
4141

42+
func (c *NetworkRuleClient) CreateIP(t *testing.T) (*sdk.NetworkRule, func()) {
43+
t.Helper()
44+
return c.CreateWithRequest(t, sdk.NewCreateNetworkRuleRequest(
45+
c.ids.RandomSchemaObjectIdentifier(),
46+
sdk.NetworkRuleTypeIpv4,
47+
[]sdk.NetworkRuleValue{{Value: "1.2.3.4"}},
48+
sdk.NetworkRuleModeIngress,
49+
))
50+
}
51+
4252
func (c *NetworkRuleClient) CreateEgressWithIdentifier(t *testing.T, id sdk.SchemaObjectIdentifier) (*sdk.NetworkRule, func()) {
4353
t.Helper()
4454
return c.CreateWithRequest(t, sdk.NewCreateNetworkRuleRequest(

pkg/sdk/poc/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ find a better solution to solve the issue (add more logic to the templates ?)
142142
- Add a possibility to generate a non-sql method with a custom implementation. Currently, it is done only in `ShowById...` functions with `newNoSqlOperation`.
143143
- improve handling operations that return one row
144144
- add more context to validated identifiers, so that error contains the affected field
145+
- add custom identifier wrapping, like it's used in security integrations' network policies
145146
146147
##### Known issues
147148
- generating two converts when Show and Desc use the same data structure

pkg/sdk/security_integrations_def.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ var oauthForCustomClientsIntegrationSetDef = g.NewQueryStruct("OauthForCustomCli
512512
g.KindOfT[OauthSecurityIntegrationUseSecondaryRolesOption](),
513513
g.ParameterOptions(),
514514
).
515-
OptionalIdentifier("NetworkPolicy", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions().Equals().SQL("NETWORK_POLICY")).
515+
OptionalTextAssignment("NETWORK_POLICY", g.ParameterOptions().NoQuotes()).
516516
OptionalTextAssignment("OAUTH_CLIENT_RSA_PUBLIC_KEY", g.ParameterOptions().SingleQuotes()).
517517
OptionalTextAssignment("OAUTH_CLIENT_RSA_PUBLIC_KEY_2", g.ParameterOptions().SingleQuotes()).
518518
OptionalComment().
@@ -567,7 +567,7 @@ var saml2IntegrationUnsetDef = g.NewQueryStruct("Saml2IntegrationUnset").
567567

568568
var scimIntegrationSetDef = g.NewQueryStruct("ScimIntegrationSet").
569569
OptionalBooleanAssignment("ENABLED", g.ParameterOptions()).
570-
OptionalIdentifier("NetworkPolicy", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions().Equals().SQL("NETWORK_POLICY")).
570+
OptionalTextAssignment("NETWORK_POLICY", g.ParameterOptions().NoQuotes()).
571571
OptionalBooleanAssignment("SYNC_PASSWORD", g.ParameterOptions()).
572572
// TODO(SNOW-1461780): use COMMENT in unset and here use OptionalComment
573573
OptionalAssignment("COMMENT", "StringAllowEmpty", g.ParameterOptions()).
@@ -733,7 +733,7 @@ var SecurityIntegrationsDef = g.NewInterface(
733733
g.KindOfT[OauthSecurityIntegrationClientTypeOption](),
734734
g.ParameterOptions().Required().SingleQuotes(),
735735
).
736-
OptionalTextAssignment("OAUTH_REDIRECT_URI", g.ParameterOptions().SingleQuotes()).
736+
TextAssignment("OAUTH_REDIRECT_URI", g.ParameterOptions().Required().SingleQuotes()).
737737
OptionalBooleanAssignment("ENABLED", g.ParameterOptions()).
738738
OptionalBooleanAssignment("OAUTH_ALLOW_NON_TLS_REDIRECT_URI", g.ParameterOptions()).
739739
OptionalBooleanAssignment("OAUTH_ENFORCE_PKCE", g.ParameterOptions()).
@@ -746,7 +746,7 @@ var SecurityIntegrationsDef = g.NewInterface(
746746
OptionalQueryStructField("BlockedRolesList", blockedRolesListDef, g.ParameterOptions().SQL("BLOCKED_ROLES_LIST").Parentheses()).
747747
OptionalBooleanAssignment("OAUTH_ISSUE_REFRESH_TOKENS", g.ParameterOptions()).
748748
OptionalNumberAssignment("OAUTH_REFRESH_TOKEN_VALIDITY", g.ParameterOptions()).
749-
OptionalIdentifier("NetworkPolicy", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions().Equals().SQL("NETWORK_POLICY")).
749+
OptionalTextAssignment("NETWORK_POLICY", g.ParameterOptions().NoQuotes()).
750750
OptionalTextAssignment("OAUTH_CLIENT_RSA_PUBLIC_KEY", g.ParameterOptions().SingleQuotes()).
751751
OptionalTextAssignment("OAUTH_CLIENT_RSA_PUBLIC_KEY_2", g.ParameterOptions().SingleQuotes())
752752
}),
@@ -802,7 +802,7 @@ var SecurityIntegrationsDef = g.NewInterface(
802802
g.KindOfT[ScimSecurityIntegrationRunAsRoleOption](),
803803
g.ParameterOptions().SingleQuotes().Required(),
804804
).
805-
OptionalIdentifier("NetworkPolicy", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions().Equals().SQL("NETWORK_POLICY")).
805+
OptionalTextAssignment("NETWORK_POLICY", g.ParameterOptions().NoQuotes()).
806806
OptionalBooleanAssignment("SYNC_PASSWORD", g.ParameterOptions())
807807
}),
808808
).

pkg/sdk/security_integrations_dto_gen.go

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,6 @@ type CreateApiAuthenticationWithClientCredentialsFlowSecurityIntegrationRequest
4040
Comment *string
4141
}
4242

43-
func (r *CreateApiAuthenticationWithClientCredentialsFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
44-
return r.name
45-
}
46-
4743
type CreateApiAuthenticationWithAuthorizationCodeGrantFlowSecurityIntegrationRequest struct {
4844
OrReplace *bool
4945
IfNotExists *bool
@@ -61,10 +57,6 @@ type CreateApiAuthenticationWithAuthorizationCodeGrantFlowSecurityIntegrationReq
6157
Comment *string
6258
}
6359

64-
func (r *CreateApiAuthenticationWithAuthorizationCodeGrantFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
65-
return r.name
66-
}
67-
6860
type CreateApiAuthenticationWithJwtBearerFlowSecurityIntegrationRequest struct {
6961
OrReplace *bool
7062
IfNotExists *bool
@@ -82,10 +74,6 @@ type CreateApiAuthenticationWithJwtBearerFlowSecurityIntegrationRequest struct {
8274
Comment *string
8375
}
8476

85-
func (r *CreateApiAuthenticationWithJwtBearerFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
86-
return r.name
87-
}
88-
8977
type CreateExternalOauthSecurityIntegrationRequest struct {
9078
OrReplace *bool
9179
IfNotExists *bool
@@ -107,10 +95,6 @@ type CreateExternalOauthSecurityIntegrationRequest struct {
10795
Comment *string
10896
}
10997

110-
func (r *CreateExternalOauthSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
111-
return r.name
112-
}
113-
11498
type BlockedRolesListRequest struct {
11599
BlockedRolesList []AccountObjectIdentifier // required
116100
}
@@ -137,10 +121,6 @@ type CreateOauthForPartnerApplicationsSecurityIntegrationRequest struct {
137121
Comment *string
138122
}
139123

140-
func (r *CreateOauthForPartnerApplicationsSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
141-
return r.name
142-
}
143-
144124
type CreateOauthForCustomClientsSecurityIntegrationRequest struct {
145125
OrReplace *bool
146126
IfNotExists *bool
@@ -161,10 +141,6 @@ type CreateOauthForCustomClientsSecurityIntegrationRequest struct {
161141
Comment *string
162142
}
163143

164-
func (r *CreateOauthForCustomClientsSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
165-
return r.name
166-
}
167-
168144
type PreAuthorizedRolesListRequest struct {
169145
PreAuthorizedRolesList []AccountObjectIdentifier
170146
}
@@ -192,10 +168,6 @@ type CreateSaml2SecurityIntegrationRequest struct {
192168
Comment *string
193169
}
194170

195-
func (r *CreateSaml2SecurityIntegrationRequest) GetName() AccountObjectIdentifier {
196-
return r.name
197-
}
198-
199171
type CreateScimSecurityIntegrationRequest struct {
200172
OrReplace *bool
201173
IfNotExists *bool
@@ -208,10 +180,6 @@ type CreateScimSecurityIntegrationRequest struct {
208180
Comment *string
209181
}
210182

211-
func (r *CreateScimSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
212-
return r.name
213-
}
214-
215183
type AlterApiAuthenticationWithClientCredentialsFlowSecurityIntegrationRequest struct {
216184
IfExists *bool
217185
name AccountObjectIdentifier // required
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package sdk
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
func (r *CreateApiAuthenticationWithClientCredentialsFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
9+
return r.name
10+
}
11+
12+
func (r *CreateApiAuthenticationWithAuthorizationCodeGrantFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
13+
return r.name
14+
}
15+
16+
func (r *CreateApiAuthenticationWithJwtBearerFlowSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
17+
return r.name
18+
}
19+
20+
func (r *CreateExternalOauthSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
21+
return r.name
22+
}
23+
24+
func (r *CreateOauthForPartnerApplicationsSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
25+
return r.name
26+
}
27+
28+
func (r *CreateOauthForCustomClientsSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
29+
return r.name
30+
}
31+
32+
func (r *CreateSaml2SecurityIntegrationRequest) GetName() AccountObjectIdentifier {
33+
return r.name
34+
}
35+
36+
func (r *CreateScimSecurityIntegrationRequest) GetName() AccountObjectIdentifier {
37+
return r.name
38+
}
39+
40+
func (s SecurityIntegrationProperty) GetName() string {
41+
return s.Name
42+
}
43+
44+
func (s SecurityIntegrationProperty) GetDefault() string {
45+
return s.Default
46+
}
47+
48+
func (s *SecurityIntegration) SubType() (string, error) {
49+
typeParts := strings.Split(s.IntegrationType, "-")
50+
if len(typeParts) < 2 {
51+
return "", fmt.Errorf("expected \"<type> - <subtype>\", got: %s", s.IntegrationType)
52+
}
53+
return strings.TrimSpace(typeParts[1]), nil
54+
}
55+
56+
func securityIntegrationNetworkPolicyQuoted(id *AccountObjectIdentifier) *string {
57+
if id == nil {
58+
return nil
59+
}
60+
// TODO(SNOW-2236323): Use a proper generation option instead.
61+
// We need to use a custom parsing here, see SNOW-1833593 for more details.
62+
return Pointer(fmt.Sprintf("'%s'", id.FullyQualifiedName()))
63+
}

0 commit comments

Comments
 (0)