diff --git a/.changelog/3288.txt b/.changelog/3288.txt new file mode 100644 index 00000000000..ef2c722a052 --- /dev/null +++ b/.changelog/3288.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/cloudflare_access_policies: added support for reusable policies +``` + +```release-note:enhancement +resource/cloudflare_access_application: added support for 'policies' argument +``` \ No newline at end of file diff --git a/go.mod b/go.mod index 1e8c12491a6..7251fa5c596 100644 --- a/go.mod +++ b/go.mod @@ -100,3 +100,7 @@ require ( google.golang.org/protobuf v1.33.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace ( + github.com/cloudflare/cloudflare-go v0.94.0 => "/home/egomes/cloudflare-go" +) \ No newline at end of file diff --git a/internal/sdkv2provider/resource_cloudflare_access_application.go b/internal/sdkv2provider/resource_cloudflare_access_application.go index 0a72dce30d6..180b7d449b9 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_application.go +++ b/internal/sdkv2provider/resource_cloudflare_access_application.go @@ -105,6 +105,10 @@ func resourceCloudflareAccessApplicationCreate(ctx context.Context, d *schema.Re } } + if policies, ok := d.GetOk("policies"); ok { + newAccessApplication.Policies = expandInterfaceToStringList(policies) + } + tflog.Debug(ctx, fmt.Sprintf("Creating Cloudflare Access Application from struct: %+v", newAccessApplication)) identifier, err := initIdentifier(d) @@ -212,6 +216,14 @@ func resourceCloudflareAccessApplicationRead(ctx context.Context, d *schema.Reso d.Set("self_hosted_domains", accessApplication.SelfHostedDomains) } + if _, ok := d.GetOk("policies"); ok { + policyIDs := make([]string, len(accessApplication.Policies)) + for i := range accessApplication.Policies { + policyIDs[i] = accessApplication.Policies[i].ID + } + d.Set("policies", policyIDs) + } + return nil } @@ -260,6 +272,11 @@ func resourceCloudflareAccessApplicationUpdate(ctx context.Context, d *schema.Re updatedAccessApplication.SelfHostedDomains = expandInterfaceToStringList(value.(*schema.Set).List()) } + if value, ok := d.GetOk("policies"); ok { + policies := expandInterfaceToStringList(value) + updatedAccessApplication.Policies = &policies + } + if _, ok := d.GetOk("cors_headers"); ok { CORSConfig, err := convertCORSSchemaToStruct(d) if err != nil { diff --git a/internal/sdkv2provider/resource_cloudflare_access_application_test.go b/internal/sdkv2provider/resource_cloudflare_access_application_test.go index 23868f72e8e..f08d2b85390 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_application_test.go +++ b/internal/sdkv2provider/resource_cloudflare_access_application_test.go @@ -694,6 +694,31 @@ func TestAccCloudflareAccessApplication_WithDefinedTags(t *testing.T) { }) } +func TestAccCloudflareAccessApplication_WithReusablePolicies(t *testing.T) { + rnd := generateRandomResourceName() + name := fmt.Sprintf("cloudflare_access_application.%s", rnd) + accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID") + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckAccount(t) + }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckCloudflareAccessApplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudflareAccessApplicationConfigWithReusablePolicies(rnd, domain, accountID), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(name, "name", rnd), + resource.TestCheckResourceAttr(name, "domain", fmt.Sprintf("%s.%s", rnd, domain)), + resource.TestCheckResourceAttr(name, "type", "self_hosted"), + resource.TestCheckResourceAttr(name, "policies.#", "2"), + ), + }, + }, + }) +} + func TestAccCloudflareAccessApplication_WithAppLauncherCustomization(t *testing.T) { rnd := generateRandomResourceName() name := fmt.Sprintf("cloudflare_access_application.%s", rnd) @@ -1311,7 +1336,40 @@ resource "cloudflare_access_application" "%[1]s" { domain = "%[1]s.%[3]s" type = "self_hosted" session_duration = "24h" - tags = [cloudflare_access_tag.%[1]s.id] + tags = [cloudflare_access_tag.%[1]s.id] } `, rnd, zoneID, domain, accountID) } + +func testAccCloudflareAccessApplicationConfigWithReusablePolicies(rnd, domain string, accountID string) string { + return fmt.Sprintf(` +resource "cloudflare_access_policy" "%[1]s_p1" { + account_id = "%[3]s" + name = "%[1]s" + decision = "allow" + include { + email = ["a@example.com"] + } +} + +resource "cloudflare_access_policy" "%[1]s_p2" { + account_id = "%[3]s" + name = "%[1]s" + decision = "non_identity" + include { + ip = ["127.0.0.1/32"] + } +} + +resource "cloudflare_access_application" "%[1]s" { + account_id = "%[3]s" + name = "%[1]s" + domain = "%[1]s.%[2]s" + type = "self_hosted" + policies = [ + cloudflare_access_policy.%[1]s_p1.id, + cloudflare_access_policy.%[1]s_p2.id + ] +} +`, rnd, domain, accountID) +} diff --git a/internal/sdkv2provider/resource_cloudflare_access_policy.go b/internal/sdkv2provider/resource_cloudflare_access_policy.go index 8a01a59e2d3..2ed3667766b 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_policy.go +++ b/internal/sdkv2provider/resource_cloudflare_access_policy.go @@ -58,29 +58,15 @@ func schemaAccessPolicyApprovalGroupToAPI(data map[string]interface{}) cloudflar return approvalGroup } -func resourceCloudflareAccessPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - client := meta.(*cloudflare.API) - appID := d.Get("application_id").(string) - - identifier, err := initIdentifier(d) - if err != nil { - return diag.FromErr(err) - } - - accessPolicy, err := client.GetAccessPolicy(ctx, identifier, cloudflare.GetAccessPolicyParams{ApplicationID: appID, PolicyID: d.Id()}) - if err != nil { - var notFoundError *cloudflare.NotFoundError - if errors.As(err, ¬FoundError) { - tflog.Info(ctx, fmt.Sprintf("Access Policy %s no longer exists", d.Id())) - d.SetId("") - return nil - } - return diag.FromErr(fmt.Errorf("error finding Access Policy %q: %w", d.Id(), err)) +func apiCloudflareAccessPolicyToResource(ctx context.Context, d *schema.ResourceData, appID string, accessPolicy cloudflare.AccessPolicy) diag.Diagnostics { + if appID != "" { + // policy is tied to a single application (legacy) and its execution precedence + // within the app can be set. + d.Set("precedence", accessPolicy.Precedence) } d.Set("name", accessPolicy.Name) d.Set("decision", accessPolicy.Decision) - d.Set("precedence", accessPolicy.Precedence) if err := d.Set("require", TransformAccessGroupForSchema(ctx, accessPolicy.Require)); err != nil { return diag.FromErr(fmt.Errorf("failed to set require attribute: %w", err)) @@ -112,17 +98,42 @@ func resourceCloudflareAccessPolicyRead(ctx context.Context, d *schema.ResourceD return diag.FromErr(fmt.Errorf("failed to set approval_group attribute: %w", err)) } } - return nil } +func resourceCloudflareAccessPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client := meta.(*cloudflare.API) + + identifier, err := initIdentifier(d) + if err != nil { + return diag.FromErr(err) + } + + appID, _ := d.Get("application_id").(string) + policy, err := client.GetAccessPolicy(ctx, identifier, cloudflare.GetAccessPolicyParams{ + ApplicationID: appID, + PolicyID: d.Id(), + }) + if err != nil { + var notFoundError *cloudflare.NotFoundError + if errors.As(err, ¬FoundError) { + tflog.Info(ctx, fmt.Sprintf("Access Policy %s no longer exists", d.Id())) + d.SetId("") + return nil + } + return diag.FromErr(fmt.Errorf("error finding Access Policy %q: %w", d.Id(), err)) + } + return apiCloudflareAccessPolicyToResource(ctx, d, appID, policy) +} + func resourceCloudflareAccessPolicyCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*cloudflare.API) - appID := d.Get("application_id").(string) + appID, _ := d.Get("application_id").(string) + precedence, _ := d.Get("precedence").(int) newAccessPolicy := cloudflare.CreateAccessPolicyParams{ ApplicationID: appID, + Precedence: precedence, Name: d.Get("name").(string), - Precedence: d.Get("precedence").(int), Decision: d.Get("decision").(string), SessionDuration: cloudflare.StringPtr(d.Get("session_duration").(string)), } @@ -175,22 +186,23 @@ func resourceCloudflareAccessPolicyCreate(ctx context.Context, d *schema.Resourc accessPolicy, err := client.CreateAccessPolicy(ctx, identifier, newAccessPolicy) if err != nil { - return diag.FromErr(fmt.Errorf("error creating Access Policy for ID %q: %w", accessPolicy.ID, err)) + return diag.FromErr(fmt.Errorf("error creating Access Policy for ID %q: %w", d.Id(), err)) } d.SetId(accessPolicy.ID) - return nil + return apiCloudflareAccessPolicyToResource(ctx, d, appID, accessPolicy) } func resourceCloudflareAccessPolicyUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*cloudflare.API) - appID := d.Get("application_id").(string) - updatedAccessPolicy := cloudflare.UpdateAccessPolicyParams{ + appID, _ := d.Get("application_id").(string) + precedence, _ := d.Get("precedence").(int) + updateReq := cloudflare.UpdateAccessPolicyParams{ ApplicationID: appID, + Precedence: precedence, PolicyID: d.Id(), Name: d.Get("name").(string), - Precedence: d.Get("precedence").(int), Decision: d.Get("decision").(string), SessionDuration: cloudflare.StringPtr(d.Get("session_duration").(string)), } @@ -198,73 +210,71 @@ func resourceCloudflareAccessPolicyUpdate(ctx context.Context, d *schema.Resourc exclude := d.Get("exclude").([]interface{}) for _, value := range exclude { if value != nil { - updatedAccessPolicy.Exclude = BuildAccessGroupCondition(value.(map[string]interface{})) + updateReq.Exclude = BuildAccessGroupCondition(value.(map[string]interface{})) } } require := d.Get("require").([]interface{}) for _, value := range require { if value != nil { - updatedAccessPolicy.Require = BuildAccessGroupCondition(value.(map[string]interface{})) + updateReq.Require = BuildAccessGroupCondition(value.(map[string]interface{})) } } include := d.Get("include").([]interface{}) for _, value := range include { if value != nil { - updatedAccessPolicy.Include = BuildAccessGroupCondition(value.(map[string]interface{})) + updateReq.Include = BuildAccessGroupCondition(value.(map[string]interface{})) } } isolationRequired := d.Get("isolation_required").(bool) - updatedAccessPolicy.IsolationRequired = &isolationRequired + updateReq.IsolationRequired = &isolationRequired purposeJustificationRequired := d.Get("purpose_justification_required").(bool) - updatedAccessPolicy.PurposeJustificationRequired = &purposeJustificationRequired + updateReq.PurposeJustificationRequired = &purposeJustificationRequired purposeJustificationPrompt := d.Get("purpose_justification_prompt").(string) - updatedAccessPolicy.PurposeJustificationPrompt = &purposeJustificationPrompt + updateReq.PurposeJustificationPrompt = &purposeJustificationPrompt approvalRequired := d.Get("approval_required").(bool) - updatedAccessPolicy.ApprovalRequired = &approvalRequired + updateReq.ApprovalRequired = &approvalRequired approvalGroups := d.Get("approval_group").([]interface{}) for _, approvalGroup := range approvalGroups { approvalGroupAsMap := approvalGroup.(map[string]interface{}) - updatedAccessPolicy.ApprovalGroups = append(updatedAccessPolicy.ApprovalGroups, schemaAccessPolicyApprovalGroupToAPI(approvalGroupAsMap)) + updateReq.ApprovalGroups = append(updateReq.ApprovalGroups, schemaAccessPolicyApprovalGroupToAPI(approvalGroupAsMap)) } - tflog.Debug(ctx, fmt.Sprintf("Updating Cloudflare Access Policy from struct: %+v", updatedAccessPolicy)) + tflog.Debug(ctx, fmt.Sprintf("Updating Cloudflare Access Policy from struct: %+v", updateReq)) identifier, err := initIdentifier(d) if err != nil { return diag.FromErr(err) } - accessPolicy, err := client.UpdateAccessPolicy(ctx, identifier, updatedAccessPolicy) + updatedPolicy, err := client.UpdateAccessPolicy(ctx, identifier, updateReq) if err != nil { return diag.FromErr(fmt.Errorf("error updating Access Policy for ID %q: %w", d.Id(), err)) } - if accessPolicy.ID == "" { - return diag.FromErr(fmt.Errorf("failed to find Access Policy ID in update response; resource was empty")) - } - - return resourceCloudflareAccessPolicyRead(ctx, d, meta) + return apiCloudflareAccessPolicyToResource(ctx, d, updateReq.ApplicationID, updatedPolicy) } func resourceCloudflareAccessPolicyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*cloudflare.API) - appID := d.Get("application_id").(string) - - tflog.Debug(ctx, fmt.Sprintf("Deleting Cloudflare Access Policy using ID: %s", d.Id())) identifier, err := initIdentifier(d) if err != nil { return diag.FromErr(err) } - err = client.DeleteAccessPolicy(ctx, identifier, cloudflare.DeleteAccessPolicyParams{ApplicationID: appID, PolicyID: d.Id()}) + appID, _ := d.Get("application_id").(string) + tflog.Debug(ctx, fmt.Sprintf("Deleting Cloudflare Access Policy using ID: %s", d.Id())) + err = client.DeleteAccessPolicy(ctx, identifier, cloudflare.DeleteAccessPolicyParams{ + ApplicationID: appID, + PolicyID: d.Id(), + }) if err != nil { return diag.FromErr(fmt.Errorf("error deleting Access Policy for ID %q: %w", d.Id(), err)) } @@ -277,23 +287,32 @@ func resourceCloudflareAccessPolicyDelete(ctx context.Context, d *schema.Resourc func resourceCloudflareAccessPolicyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { attributes := strings.SplitN(d.Id(), "/", 4) - if len(attributes) != 4 { + if len(attributes) < 3 { return nil, fmt.Errorf( - "invalid id (%q) specified, should be in format %q or %q", + "invalid id (%q) specified, should be in format %q, %q or %q", d.Id(), + "account/accountID/accessPolicyID", "account/accountID/accessApplicationID/accessPolicyID", "zone/zoneID/accessApplicationID/accessPolicyID", ) } - identifierType, identifierID, accessAppID, accessPolicyID := attributes[0], attributes[1], attributes[2], attributes[3] - - tflog.Debug(ctx, fmt.Sprintf("Importing Cloudflare Access Policy: %s %q, appID %q, accessPolicyID %q", identifierType, identifierID, accessAppID, accessPolicyID)) + identifierType, identifierID := attributes[0], attributes[1] + if len(attributes) == 4 { + // Legacy policy tied to a single application. + accessAppID, accessPolicyID := attributes[2], attributes[3] + tflog.Debug(ctx, fmt.Sprintf("Importing Cloudflare Access Policy: %s %q, appID %q, accessPolicyID %q", identifierType, identifierID, accessAppID, accessPolicyID)) + d.Set("application_id", accessAppID) + d.SetId(accessPolicyID) + } else { + // Standalone reusable policy + accessPolicyID := attributes[2] + tflog.Debug(ctx, fmt.Sprintf("Importing Cloudflare Access Policy: %s %q, accessPolicyID %q", identifierType, identifierID, accessPolicyID)) + d.SetId(accessPolicyID) + } //lintignore:R001 d.Set(fmt.Sprintf("%s_id", identifierType), identifierID) - d.Set("application_id", accessAppID) - d.SetId(accessPolicyID) resourceCloudflareAccessPolicyRead(ctx, d, meta) diff --git a/internal/sdkv2provider/resource_cloudflare_access_policy_test.go b/internal/sdkv2provider/resource_cloudflare_access_policy_test.go index ed4bbed99b5..1fc3d748c80 100644 --- a/internal/sdkv2provider/resource_cloudflare_access_policy_test.go +++ b/internal/sdkv2provider/resource_cloudflare_access_policy_test.go @@ -823,6 +823,29 @@ func TestAccCloudflareAccessPolicy_ApprovalGroup(t *testing.T) { }) } +func TestAccCloudflareAccessPolicy_Reusable(t *testing.T) { + rnd := generateRandomResourceName() + name := "cloudflare_access_policy." + rnd + accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckAccount(t) + }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: testAccessPolicyReusableConfig(rnd, accountID), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(name, "name", rnd), + resource.TestCheckResourceAttr(name, "include.0.email.0", "a@example.com"), + ), + }, + }, + }) +} + func testAccCheckCloudflareAccessPolicyHasApprovalGroups(n string, accessIdentifier *cloudflare.ResourceContainer) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -1057,3 +1080,16 @@ func testAccessPolicyIsolationRequiredConfig(resourceID, zone, accountID string) `, resourceID, zone, accountID) } + +func testAccessPolicyReusableConfig(resourceID, accountID string) string { + return fmt.Sprintf(` + resource "cloudflare_access_policy" "%[1]s" { + name = "%[1]s" + account_id = "%[2]s" + decision = "allow" + include { + email = ["a@example.com"] + } + } + `, resourceID, accountID) +} diff --git a/internal/sdkv2provider/schema_cloudflare_access_application.go b/internal/sdkv2provider/schema_cloudflare_access_application.go index 06054358d85..c7450dd1eba 100644 --- a/internal/sdkv2provider/schema_cloudflare_access_application.go +++ b/internal/sdkv2provider/schema_cloudflare_access_application.go @@ -59,6 +59,17 @@ func resourceCloudflareAccessApplicationSchema() map[string]*schema.Schema { ValidateFunc: validation.StringInSlice([]string{"app_launcher", "bookmark", "biso", "dash_sso", "saas", "self_hosted", "ssh", "vnc", "warp"}, false), Description: fmt.Sprintf("The application type. %s", renderAvailableDocumentationValuesStringSlice([]string{"app_launcher", "bookmark", "biso", "dash_sso", "saas", "self_hosted", "ssh", "vnc", "warp"})), }, + "policies": { + Type: schema.TypeList, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Optional: true, + Description: "The policies associated with the application, in ascending order of precedence." + + " When omitted, the application policies are not be updated." + + " Warning: Do not use this field while you still have this application ID referenced as `application_id`" + + " in an `cloudflare_access_policy` resource, as it can result in an inconsistent state.", + }, "session_duration": { Type: schema.TypeString, Optional: true, diff --git a/internal/sdkv2provider/schema_cloudflare_access_policy.go b/internal/sdkv2provider/schema_cloudflare_access_policy.go index 7ae37e1420c..ff73bafaa35 100644 --- a/internal/sdkv2provider/schema_cloudflare_access_policy.go +++ b/internal/sdkv2provider/schema_cloudflare_access_policy.go @@ -12,23 +12,25 @@ import ( func resourceCloudflareAccessPolicySchema() map[string]*schema.Schema { return map[string]*schema.Schema{ "application_id": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Deprecated: "This field is deprecated. Policies can now be standalone and reusable by multiple applications." + + "Please use `cloudflare_access_application.policies` to associate policies with applications.", Description: "The ID of the application the policy is associated with.", }, consts.AccountIDSchemaKey: { - Description: consts.AccountIDSchemaDescription, - Type: schema.TypeString, - Optional: true, - Computed: true, - ConflictsWith: []string{consts.ZoneIDSchemaKey}, + Description: consts.AccountIDSchemaDescription, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ExactlyOneOf: []string{consts.AccountIDSchemaKey, consts.ZoneIDSchemaKey}, }, consts.ZoneIDSchemaKey: { - Description: consts.ZoneIDSchemaDescription, - Type: schema.TypeString, - Optional: true, - Computed: true, - ConflictsWith: []string{consts.AccountIDSchemaKey}, + Description: consts.ZoneIDSchemaDescription, + Type: schema.TypeString, + Optional: true, + ForceNew: true, }, "name": { Type: schema.TypeString, @@ -36,8 +38,12 @@ func resourceCloudflareAccessPolicySchema() map[string]*schema.Schema { Description: "Friendly name of the Access Policy.", }, "precedence": { - Type: schema.TypeInt, - Required: true, + Type: schema.TypeInt, + Optional: true, + RequiredWith: []string{"application_id"}, + Deprecated: "This field is deprecated. Access policies can now be reusable by multiple applications." + + " Please use `cloudflare_access_application.policies` to link policies to an application with" + + " ascending order of precedence.", Description: "The unique precedence for policies on a single application.", }, "decision": { diff --git a/templates/resources/access_policy.md.tmpl b/templates/resources/access_policy.md.tmpl index 049f3f5361e..b7ea88ab5e3 100644 --- a/templates/resources/access_policy.md.tmpl +++ b/templates/resources/access_policy.md.tmpl @@ -9,11 +9,13 @@ description: |- {{ .Description | trimspace }} -~> It's required that an `account_id` or `zone_id` is provided and in - most cases using either is fine. However, if you're using a scoped - access token, you must provide the argument that matches the token's - scope. For example, an access token that is scoped to the "example.com" - zone needs to use the `zone_id` argument. +~> It's required that an `account_id` or `zone_id` is provided and in most cases using either is fine. + However, if you're using a scoped access token, you must provide the argument that matches the token's + scope. For example, an access token that is scoped to the "example.com" zone needs to use the `zone_id` argument. + If 'application_id' is omitted, the policy created can be reused by multiple access applications. + Any access_application resource can reference reusable policies through its 'policies' argument. + To destroy a reusable policy and remove it from all applications' policies lists on the same apply, preemptively set the + lifecycle option 'create_before_destroy' to true on the 'access_policy' resource. {{ if .HasExample -}} ## Example Usage