From 11a702246dd068db28114d2585e7d441a7715992 Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Tue, 20 Jun 2023 12:03:57 +0300 Subject: [PATCH 1/7] [FWaaS_V2]: Add policies Add data_source_openstack_fw_policy_v2 with tests. Add import_openstack_fw_policy_v2_test. Add resource_openstack_fw_policy_v2 with tests. Add policy_v2 docs. Add policy_v2 to sources maps. --- .../data_source_openstack_fw_policy_v2.go | 134 ++++++++++++ ...data_source_openstack_fw_policy_v2_test.go | 108 ++++++++++ openstack/fw_policy_v2.go | 27 +++ .../import_openstack_fw_policy_v2_test.go | 32 +++ openstack/provider.go | 2 + openstack/resource_openstack_fw_policy_v2.go | 201 ++++++++++++++++++ .../resource_openstack_fw_policy_v2_test.go | 187 ++++++++++++++++ website/docs/d/fw_policy_v2.html.markdown | 50 +++++ website/docs/r/fw_policy_v2.html.markdown | 103 +++++++++ website/openstack.erb | 8 +- 10 files changed, 851 insertions(+), 1 deletion(-) create mode 100644 openstack/data_source_openstack_fw_policy_v2.go create mode 100644 openstack/data_source_openstack_fw_policy_v2_test.go create mode 100644 openstack/fw_policy_v2.go create mode 100644 openstack/import_openstack_fw_policy_v2_test.go create mode 100644 openstack/resource_openstack_fw_policy_v2.go create mode 100644 openstack/resource_openstack_fw_policy_v2_test.go create mode 100644 website/docs/d/fw_policy_v2.html.markdown create mode 100644 website/docs/r/fw_policy_v2.html.markdown diff --git a/openstack/data_source_openstack_fw_policy_v2.go b/openstack/data_source_openstack_fw_policy_v2.go new file mode 100644 index 000000000..b2e35fe37 --- /dev/null +++ b/openstack/data_source_openstack_fw_policy_v2.go @@ -0,0 +1,134 @@ +package openstack + +import ( + "context" + "log" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/policies" +) + +func dataSourceFWPolicyV2() *schema.Resource { + return &schema.Resource{ + ReadContext: dataSourceFWPolicyV2Read, + + Schema: map[string]*schema.Schema{ + "region": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "name": { + Type: schema.TypeString, + Optional: true, + }, + + "policy_id": { + Type: schema.TypeString, + Optional: true, + }, + + "tenant_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + }, + + "description": { + Type: schema.TypeString, + Optional: true, + }, + + "shared": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + }, + + "audited": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + }, + + "rules": { + Type: schema.TypeList, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + } +} + +func dataSourceFWPolicyV2Read(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + config := meta.(*Config) + networkingClient, err := config.NetworkingV2Client(GetRegion(d, config)) + if err != nil { + return diag.Errorf("Error creating OpenStack networking client: %s", err) + } + + listOpts := policies.ListOpts{} + + if v, ok := d.GetOk("name"); ok { + listOpts.Name = v.(string) + } + + if v, ok := d.GetOk("description"); ok { + listOpts.Description = v.(string) + } + + if v, ok := d.GetOk("policy_id"); ok { + listOpts.ID = v.(string) + } + + if v, ok := d.GetOk("tenant_id"); ok { + listOpts.TenantID = v.(string) + } + + if v, ok := d.GetOk("shared"); ok { + listOpts.Shared = v.(*bool) + } + + if v, ok := d.GetOk("audited"); ok { + listOpts.Audited = v.(*bool) + } + + pages, err := policies.List(networkingClient, listOpts).AllPages() + if err != nil { + return diag.FromErr(err) + } + + allFWPolicies, err := policies.ExtractPolicies(pages) + if err != nil { + return diag.Errorf("Unable to retrieve openstack_fw_policy_v2: %s", err) + } + + if len(allFWPolicies) < 1 { + return diag.Errorf("Your openstack_fw_policy_v2 query returned no results") + } + + if len(allFWPolicies) > 1 { + return diag.Errorf("Your openstack_fw_policy_v2 query returned more than one result") + } + + policy := allFWPolicies[0] + + log.Printf("[DEBUG] Retrieved openstack_fw_policy_v2 %s: %#v", policy.ID, policy) + + d.SetId(policy.ID) + + d.Set("name", policy.Name) + d.Set("tenant_id", policy.TenantID) + d.Set("description", policy.Description) + d.Set("shared", policy.Shared) + d.Set("audited", policy.Audited) + d.Set("rules", policy.Rules) + d.Set("region", GetRegion(d, config)) + + return nil +} diff --git a/openstack/data_source_openstack_fw_policy_v2_test.go b/openstack/data_source_openstack_fw_policy_v2_test.go new file mode 100644 index 000000000..6e6ef9fec --- /dev/null +++ b/openstack/data_source_openstack_fw_policy_v2_test.go @@ -0,0 +1,108 @@ +package openstack + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +func TestAccFWPolicyV2DataSource_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + }, + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2DataSourceBasic, + }, + { + Config: testAccFWPolicyV2DataSourceName(), + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2DataSourceID("data.openstack_fw_policy_v2.policy_1"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "name", "policy_1"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "description", "My firewall policy"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "shared", "false"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "audited", "false"), + resource.TestCheckResourceAttrSet( + "data.openstack_fw_policy_v2.policy_1", "tenant_id"), + ), + }, + }, + }) +} +func TestAccFWPolicyV2DataSource_FWPolicyID(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + }, + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2DataSourceBasic, + }, + { + Config: testAccFWPolicyV2DataSourcePolicyID(), + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2DataSourceID("data.openstack_fw_policy_v2.policy_1"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "name", "policy_1"), + ), + }, + }, + }) +} + +func testAccCheckFWPolicyV2DataSourceID(n string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Can't find firewall policy data source: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("Firewall policy data source ID not set") + } + + return nil + } +} + +const testAccFWPolicyV2DataSourceBasic = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "My firewall policy" +} +` + +func testAccFWPolicyV2DataSourceName() string { + return fmt.Sprintf(` +%s + +data "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "My firewall policy" + tenant_id = "${openstack_fw_policy_v2.policy_1.tenant_id}" + shared = false + audited = false +} +`, testAccFWPolicyV2DataSourceBasic) +} + +func testAccFWPolicyV2DataSourcePolicyID() string { + return fmt.Sprintf(` +%s + +data "openstack_fw_policy_v2" "policy_1" { + policy_id = "${openstack_fw_policy_v2.policy_1.id}" +} +`, testAccFWPolicyV2DataSourceBasic) +} diff --git a/openstack/fw_policy_v2.go b/openstack/fw_policy_v2.go new file mode 100644 index 000000000..9a3cca667 --- /dev/null +++ b/openstack/fw_policy_v2.go @@ -0,0 +1,27 @@ +package openstack + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/policies" +) + +func fwPolicyV2DeleteFunc(networkingClient *gophercloud.ServiceClient, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + err := policies.Delete(networkingClient, id).Err + if err == nil { + return "", "DELETED", nil + } + + if _, ok := err.(gophercloud.ErrDefault409); ok { + // This error usually means that the policy is attached + // to a firewall. At this point, the firewall is probably + // being delete. So, we retry a few times. + + return nil, "ACTIVE", nil + } + + return nil, "ACTIVE", err + } +} diff --git a/openstack/import_openstack_fw_policy_v2_test.go b/openstack/import_openstack_fw_policy_v2_test.go new file mode 100644 index 000000000..8668b670c --- /dev/null +++ b/openstack/import_openstack_fw_policy_v2_test.go @@ -0,0 +1,32 @@ +package openstack + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func TestAccFWPolicyV2_importBasic(t *testing.T) { + resourceName := "openstack_fw_policy_v2.policy_1" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2AddRules, + }, + + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} diff --git a/openstack/provider.go b/openstack/provider.go index 2d5455b7b..b35b9d835 100644 --- a/openstack/provider.go +++ b/openstack/provider.go @@ -284,6 +284,7 @@ func Provider() *schema.Provider { "openstack_containerinfra_cluster_v1": dataSourceContainerInfraCluster(), "openstack_dns_zone_v2": dataSourceDNSZoneV2(), "openstack_fw_policy_v1": dataSourceFWPolicyV1(), + "openstack_fw_policy_v2": dataSourceFWPolicyV2(), "openstack_fw_rule_v2": dataSourceFWRuleV2(), "openstack_identity_role_v3": dataSourceIdentityRoleV3(), "openstack_identity_project_v3": dataSourceIdentityProjectV3(), @@ -355,6 +356,7 @@ func Provider() *schema.Provider { "openstack_dns_transfer_accept_v2": resourceDNSTransferAcceptV2(), "openstack_fw_firewall_v1": resourceFWFirewallV1(), "openstack_fw_policy_v1": resourceFWPolicyV1(), + "openstack_fw_policy_v2": resourceFWPolicyV2(), "openstack_fw_rule_v1": resourceFWRuleV1(), "openstack_fw_rule_v2": resourceFWRuleV2(), "openstack_identity_endpoint_v3": resourceIdentityEndpointV3(), diff --git a/openstack/resource_openstack_fw_policy_v2.go b/openstack/resource_openstack_fw_policy_v2.go new file mode 100644 index 000000000..8eee23dcd --- /dev/null +++ b/openstack/resource_openstack_fw_policy_v2.go @@ -0,0 +1,201 @@ +package openstack + +import ( + "context" + "log" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/policies" +) + +func resourceFWPolicyV2() *schema.Resource { + return &schema.Resource{ + CreateContext: resourceFWPolicyV2Create, + ReadContext: resourceFWPolicyV2Read, + UpdateContext: resourceFWPolicyV2Update, + DeleteContext: resourceFWPolicyV2Delete, + Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, + }, + + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(10 * time.Minute), + Delete: schema.DefaultTimeout(10 * time.Minute), + }, + + Schema: map[string]*schema.Schema{ + "region": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "name": { + Type: schema.TypeString, + Optional: true, + }, + + "description": { + Type: schema.TypeString, + Optional: true, + }, + + "tenant_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + }, + + "audited": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + + "shared": { + Type: schema.TypeBool, + Optional: true, + }, + + "rules": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + + "value_specs": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + }, + }, + } +} + +func resourceFWPolicyV2Create(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + config := meta.(*Config) + networkingClient, err := config.NetworkingV2Client(GetRegion(d, config)) + if err != nil { + return diag.Errorf("Error creating OpenStack networking client: %s", err) + } + + audited := d.Get("audited").(bool) + opts := policies.CreateOpts{ + Name: d.Get("name").(string), + Description: d.Get("description").(string), + Audited: &audited, + TenantID: d.Get("tenant_id").(string), + FirewallRules: expandToStringSlice(d.Get("rules").([]interface{})), + } + + if r, ok := d.GetOk("shared"); ok { + shared := r.(bool) + opts.Shared = &shared + } + + log.Printf("[DEBUG] openstack_fw_policy_v2 create options: %#v", opts) + + policy, err := policies.Create(networkingClient, opts).Extract() + if err != nil { + return diag.Errorf("Error creating openstack_fw_policy_v2: %s", err) + } + + log.Printf("[DEBUG] openstack_fw_policy_v2 %s created: %#v", policy.ID, policy) + + d.SetId(policy.ID) + + return resourceFWPolicyV2Read(ctx, d, meta) +} + +func resourceFWPolicyV2Read(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + config := meta.(*Config) + networkingClient, err := config.NetworkingV2Client(GetRegion(d, config)) + if err != nil { + return diag.Errorf("Error creating OpenStack networking client: %s", err) + } + + policy, err := policies.Get(networkingClient, d.Id()).Extract() + if err != nil { + return diag.FromErr(CheckDeleted(d, err, "Error retrieving openstack_fw_policy_v2")) + } + + log.Printf("[DEBUG] Retrieved openstack_fw_policy_v2 %s: %#v", d.Id(), policy) + + d.Set("name", policy.Name) + d.Set("description", policy.Description) + d.Set("tenant_id", policy.TenantID) + d.Set("rules", policy.Rules) + d.Set("audited", policy.Audited) + d.Set("shared", policy.Shared) + d.Set("region", GetRegion(d, config)) + + return nil +} + +func resourceFWPolicyV2Update(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + config := meta.(*Config) + networkingClient, err := config.NetworkingV2Client(GetRegion(d, config)) + if err != nil { + return diag.Errorf("Error creating OpenStack networking client: %s", err) + } + + var updateOpts policies.UpdateOpts + + if d.HasChange("name") { + name := d.Get("name").(string) + updateOpts.Name = &name + } + + if d.HasChange("description") { + description := d.Get("description").(string) + updateOpts.Description = &description + } + + if d.HasChange("rules") { + rules := expandToStringSlice(d.Get("rules").([]interface{})) + updateOpts.FirewallRules = &rules + } + + log.Printf("[DEBUG] openstack_fw_policy_v2 %s update options: %#v", d.Id(), updateOpts) + + err = policies.Update(networkingClient, d.Id(), updateOpts).Err + if err != nil { + return diag.Errorf("Error updating openstack_fw_policy_v2 %s: %s", d.Id(), err) + } + + return resourceFWPolicyV2Read(ctx, d, meta) +} + +func resourceFWPolicyV2Delete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + config := meta.(*Config) + networkingClient, err := config.NetworkingV2Client(GetRegion(d, config)) + if err != nil { + return diag.Errorf("Error creating OpenStack networking client: %s", err) + } + + _, err = policies.Get(networkingClient, d.Id()).Extract() + if err != nil { + return diag.FromErr(CheckDeleted(d, err, "Error retrieving openstack_fw_policy_v2")) + } + + stateConf := &resource.StateChangeConf{ + Pending: []string{"ACTIVE"}, + Target: []string{"DELETED"}, + Refresh: fwPolicyV2DeleteFunc(networkingClient, d.Id()), + Timeout: d.Timeout(schema.TimeoutDelete), + Delay: 0, + MinTimeout: 2 * time.Second, + } + + if _, err = stateConf.WaitForStateContext(ctx); err != nil { + return diag.Errorf("Error waiting for openstack_fw_policy_v2 %s to be deleted: %s", d.Id(), err) + } + + return nil +} diff --git a/openstack/resource_openstack_fw_policy_v2_test.go b/openstack/resource_openstack_fw_policy_v2_test.go new file mode 100644 index 000000000..723774193 --- /dev/null +++ b/openstack/resource_openstack_fw_policy_v2_test.go @@ -0,0 +1,187 @@ +package openstack + +import ( + "fmt" + "testing" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/policies" +) + +func TestAccFWPolicyV2_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2Basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", "", "", 0), + ), + }, + }, + }) +} + +func TestAccFWPolicyV2_addRules(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2AddRules, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", "policy_1", "terraform acceptance test", 2), + ), + }, + }, + }) +} + +func TestAccFWPolicyV2_deleteRules(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2DeleteRules, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", "policy_1", "terraform acceptance test", 1), + ), + }, + }, + }) +} + +func testAccCheckFWPolicyV2Destroy(s *terraform.State) error { + config := testAccProvider.Meta().(*Config) + networkingClient, err := config.NetworkingV2Client(osRegionName) + if err != nil { + return fmt.Errorf("Error creating OpenStack networking client: %s", err) + } + for _, rs := range s.RootModule().Resources { + if rs.Type != "openstack_fw_policy_v2" { + continue + } + _, err = policies.Get(networkingClient, rs.Primary.ID).Extract() + if err == nil { + return fmt.Errorf("Firewall policy (%s) still exists", rs.Primary.ID) + } + if _, ok := err.(gophercloud.ErrDefault404); !ok { + return err + } + } + return nil +} + +func testAccCheckFWPolicyV2Exists(n, name, description string, ruleCount int) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + + config := testAccProvider.Meta().(*Config) + networkingClient, err := config.NetworkingV2Client(osRegionName) + if err != nil { + return fmt.Errorf("Error creating OpenStack networking client: %s", err) + } + + var found *policies.Policy + for i := 0; i < 5; i++ { + // Firewall policy creation is asynchronous. Retry some times + // if we get a 404 error. Fail on any other error. + found, err = policies.Get(networkingClient, rs.Primary.ID).Extract() + if err != nil { + if _, ok := err.(gophercloud.ErrDefault404); ok { + time.Sleep(time.Second) + continue + } + return err + } + break + } + + switch { + case name != found.Name: + err = fmt.Errorf("Expected name <%s>, but found <%s>", name, found.Name) + case description != found.Description: + err = fmt.Errorf("Expected description <%s>, but found <%s>", description, found.Description) + case ruleCount != len(found.Rules): + err = fmt.Errorf("Expected rule count <%d>, but found <%d>", ruleCount, len(found.Rules)) + } + + if err != nil { + return err + } + + return nil + } +} + +const testAccFWPolicyV2Basic = ` +resource "openstack_fw_policy_v2" "policy_1" { +} +` + +const testAccFWPolicyV2AddRules = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.udp_deny.id}", + "${openstack_fw_rule_v2.tcp_allow.id}" + ] +} + +resource "openstack_fw_rule_v2" "tcp_allow" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "udp_deny" { + protocol = "udp" + action = "deny" +} +` + +const testAccFWPolicyV2DeleteRules = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.udp_deny.id}" + ] +} + +resource "openstack_fw_rule_v2" "udp_deny" { + protocol = "udp" + action = "deny" +} +` diff --git a/website/docs/d/fw_policy_v2.html.markdown b/website/docs/d/fw_policy_v2.html.markdown new file mode 100644 index 000000000..d69bc94de --- /dev/null +++ b/website/docs/d/fw_policy_v2.html.markdown @@ -0,0 +1,50 @@ +--- +subcategory: "Networking / Neutron" +layout: "openstack" +page_title: "OpenStack: openstack_fw_policy_v2" +sidebar_current: "docs-openstack-datasource-fw-policy-v2" +description: |- + Get information on an OpenStack Firewall Policy V2. +--- + +# openstack\_fw\_policy\_v2 + +Use this data source to get information of an available OpenStack firewall policy v2. + +## Example Usage + +```hcl +data "openstack_fw_policy_v2" "policy" { + name = "tf_test_policy" +} +``` + +## Argument Reference + +* `region` - (Optional) The region in which to obtain the V2 Neutron client. + A Neutron client is needed to retrieve firewall policy ids. If omitted, the + `region` argument of the provider is used. + +* `name` - (Optional) The name of the firewall policy. + +* `description` - (Optional) Human-readable description of the policy. + +* `policy_id` - (Optional) The ID of the firewall policy. + +* `tenant_id` - (Optional) The owner of the firewall policy. + +* `shared` - (Optional) Whether this policy is shared across all projects. + +* `audited` - (Optional) Whether this policy has been audited. + +## Attributes Reference + +The following attributes are exported: + +* `region` - See Argument Reference above. +* `name` - See Argument Reference above. +* `policy_id` - See Argument Reference above. +* `tenant_id` - See Argument Reference above. +* `shared` - The sharing status of the firewall policy. +* `audited` - The audit status of the firewall policy. +* `rules` - The array of one or more firewall rules that comprise the policy. diff --git a/website/docs/r/fw_policy_v2.html.markdown b/website/docs/r/fw_policy_v2.html.markdown new file mode 100644 index 000000000..20d03c375 --- /dev/null +++ b/website/docs/r/fw_policy_v2.html.markdown @@ -0,0 +1,103 @@ +--- +subcategory: "Networking / Neutron" +layout: "openstack" +page_title: "OpenStack: openstack_fw_policy_v2" +sidebar_current: "docs-openstack-resource-fw-policy-v2" +description: |- + Manages a v2 firewall policy resource within OpenStack. +--- + +# openstack\_fw\_policy\_v2 + +Manages a v2 firewall policy resource within OpenStack. + +~> **Note:** Firewall v2 has no support for OVN currently. + +## Example Usage + +```hcl +resource "openstack_fw_rule_v2" "rule_1" { + name = "firewall_rule_1" + description = "drop TELNET traffic" + action = "deny" + protocol = "tcp" + destination_port = "23" + enabled = "true" +} + +resource "openstack_fw_rule_v2" "rule_2" { + name = "firewall_rule_2" + description = "drop NTP traffic" + action = "deny" + protocol = "udp" + destination_port = "123" + enabled = "false" +} + +resource "openstack_fw_policy_v2" "policy_1" { + name = "firewall_policy" + + rules = [ + "${openstack_fw_rule_v2.rule_1.id}", + "${openstack_fw_rule_v2.rule_2.id}", + ] +} +``` + +## Argument Reference + +The following arguments are supported: + +* `region` - (Optional) The region in which to obtain the v2 networking client. + A networking client is needed to create a firewall policy. If omitted, the + `region` argument of the provider is used. Changing this creates a new + firewall policy. + +* `name` - (Optional) A name for the firewall policy. Changing this + updates the `name` of an existing firewall policy. + +* `description` - (Optional) A description for the firewall policy. Changing + this updates the `description` of an existing firewall policy. + +* `tenant_id` - (Optional) The owner of the firewall policy. Required if admin + wants to create a firewall policy for another tenant. Changing this + creates a new firewall policy. + +* `rules` - (Optional) An array of one or more firewall rules that comprise + the policy. Changing this results in adding/removing rules from the + existing firewall policy. + +* `audited` - (Optional) Audit status of the firewall policy + (must be "true" or "false" if provided - defaults to "false"). + This status is set to "false" whenever the firewall policy or any of its + rules are changed. Changing this updates the `audited` status of an existing + firewall policy. + +* `shared` - (Optional) Sharing status of the firewall policy (must be "true" + or "false" if provided). If this is "true" the policy is visible to, and + can be used in, firewalls in other tenants. Changing this updates the + `shared` status of an existing firewall policy. Only administrative users + can specify if the policy should be shared. + +* `value_specs` - (Optional) Map of additional options. Changing this creates a + new firewall policy. + +## Attributes Reference + +The following attributes are exported: + +* `region` - See Argument Reference above. +* `name` - See Argument Reference above. +* `tenant_id` - See Argument Reference above. +* `description` - See Argument Reference above. +* `rules` - See Argument Reference above. +* `audited` - See Argument Reference above. +* `shared` - See Argument Reference above. + +## Import + +Firewall Policies can be imported using the `id`, e.g. + +``` +$ terraform import openstack_fw_policy_v2.policy_1 07f422e6-c596-474b-8b94-fe2c12506ce0 +``` diff --git a/website/openstack.erb b/website/openstack.erb index e67de4914..d0f29258e 100644 --- a/website/openstack.erb +++ b/website/openstack.erb @@ -61,8 +61,11 @@ > openstack_fw_policy_v1 + > + openstack_fw_policy_v2 + > - openstack_fw_rule_v2 + openstack_fw_rule_v2 > openstack_identity_auth_scope_v3 @@ -465,6 +468,9 @@ > openstack_fw_policy_v1 + > + openstack_fw_policy_v2 + > openstack_fw_rule_v1 From 721747e350d6794a657f0084b91a8dae6d4fa564 Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Wed, 21 Jun 2023 11:21:51 +0300 Subject: [PATCH 2/7] Add testAccPreCheckFW in datasources, TestAccFWPolicyV2_rulesOrder, TestAccFWPolicyV2_shared, TestAccFWRuleV2_shared, fix indents, remove unused value_specs --- ...data_source_openstack_fw_policy_v2_test.go | 18 +- .../data_source_openstack_fw_rule_v2_test.go | 4 +- openstack/resource_openstack_fw_policy_v2.go | 24 +- .../resource_openstack_fw_policy_v2_test.go | 381 ++++++++++++++++-- openstack/resource_openstack_fw_rule_v2.go | 28 +- .../resource_openstack_fw_rule_v2_test.go | 181 +++++---- 6 files changed, 516 insertions(+), 120 deletions(-) diff --git a/openstack/data_source_openstack_fw_policy_v2_test.go b/openstack/data_source_openstack_fw_policy_v2_test.go index 6e6ef9fec..f8a2e8a87 100644 --- a/openstack/data_source_openstack_fw_policy_v2_test.go +++ b/openstack/data_source_openstack_fw_policy_v2_test.go @@ -13,6 +13,7 @@ func TestAccFWPolicyV2DataSource_basic(t *testing.T) { PreCheck: func() { testAccPreCheck(t) testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) }, ProviderFactories: testAccProviders, Steps: []resource.TestStep{ @@ -43,6 +44,7 @@ func TestAccFWPolicyV2DataSource_FWPolicyID(t *testing.T) { PreCheck: func() { testAccPreCheck(t) testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) }, ProviderFactories: testAccProviders, Steps: []resource.TestStep{ @@ -78,8 +80,8 @@ func testAccCheckFWPolicyV2DataSourceID(n string) resource.TestCheckFunc { const testAccFWPolicyV2DataSourceBasic = ` resource "openstack_fw_policy_v2" "policy_1" { - name = "policy_1" - description = "My firewall policy" + name = "policy_1" + description = "My firewall policy" } ` @@ -88,11 +90,11 @@ func testAccFWPolicyV2DataSourceName() string { %s data "openstack_fw_policy_v2" "policy_1" { - name = "policy_1" - description = "My firewall policy" - tenant_id = "${openstack_fw_policy_v2.policy_1.tenant_id}" - shared = false - audited = false + name = "policy_1" + description = "My firewall policy" + tenant_id = "${openstack_fw_policy_v2.policy_1.tenant_id}" + shared = false + audited = false } `, testAccFWPolicyV2DataSourceBasic) } @@ -102,7 +104,7 @@ func testAccFWPolicyV2DataSourcePolicyID() string { %s data "openstack_fw_policy_v2" "policy_1" { - policy_id = "${openstack_fw_policy_v2.policy_1.id}" + policy_id = "${openstack_fw_policy_v2.policy_1.id}" } `, testAccFWPolicyV2DataSourceBasic) } diff --git a/openstack/data_source_openstack_fw_rule_v2_test.go b/openstack/data_source_openstack_fw_rule_v2_test.go index d7fa0d789..7494cc751 100644 --- a/openstack/data_source_openstack_fw_rule_v2_test.go +++ b/openstack/data_source_openstack_fw_rule_v2_test.go @@ -13,6 +13,7 @@ func TestAccFWRuleV2DataSource_basic(t *testing.T) { PreCheck: func() { testAccPreCheck(t) testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) }, ProviderFactories: testAccProviders, Steps: []resource.TestStep{ @@ -56,6 +57,7 @@ func TestAccFWRuleV2DataSource_FWRuleID(t *testing.T) { PreCheck: func() { testAccPreCheck(t) testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) }, ProviderFactories: testAccProviders, Steps: []resource.TestStep{ @@ -127,7 +129,7 @@ func testAccFWRuleV2DataSourceRuleID() string { %s data "openstack_fw_rule_v2" "rule_1" { - rule_id = "${openstack_fw_rule_v2.rule_1.id}" + rule_id = "${openstack_fw_rule_v2.rule_1.id}" } `, testAccFWRuleV2DataSourceBasic) } diff --git a/openstack/resource_openstack_fw_policy_v2.go b/openstack/resource_openstack_fw_policy_v2.go index 8eee23dcd..5ff17d914 100644 --- a/openstack/resource_openstack_fw_policy_v2.go +++ b/openstack/resource_openstack_fw_policy_v2.go @@ -68,12 +68,6 @@ func resourceFWPolicyV2() *schema.Resource { Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - - "value_specs": { - Type: schema.TypeMap, - Optional: true, - ForceNew: true, - }, }, } } @@ -145,28 +139,36 @@ func resourceFWPolicyV2Update(ctx context.Context, d *schema.ResourceData, meta return diag.Errorf("Error creating OpenStack networking client: %s", err) } - var updateOpts policies.UpdateOpts + var ( + hasChange bool + updateOpts policies.UpdateOpts + ) if d.HasChange("name") { + hasChange = true name := d.Get("name").(string) updateOpts.Name = &name } if d.HasChange("description") { + hasChange = true description := d.Get("description").(string) updateOpts.Description = &description } if d.HasChange("rules") { + hasChange = true rules := expandToStringSlice(d.Get("rules").([]interface{})) updateOpts.FirewallRules = &rules } - log.Printf("[DEBUG] openstack_fw_policy_v2 %s update options: %#v", d.Id(), updateOpts) + if hasChange { + log.Printf("[DEBUG] openstack_fw_policy_v2 %s update options: %#v", d.Id(), updateOpts) - err = policies.Update(networkingClient, d.Id(), updateOpts).Err - if err != nil { - return diag.Errorf("Error updating openstack_fw_policy_v2 %s: %s", d.Id(), err) + err = policies.Update(networkingClient, d.Id(), updateOpts).Err + if err != nil { + return diag.Errorf("Error updating openstack_fw_policy_v2 %s: %s", d.Id(), err) + } } return resourceFWPolicyV2Read(ctx, d, meta) diff --git a/openstack/resource_openstack_fw_policy_v2_test.go b/openstack/resource_openstack_fw_policy_v2_test.go index 723774193..67ae6bf88 100644 --- a/openstack/resource_openstack_fw_policy_v2_test.go +++ b/openstack/resource_openstack_fw_policy_v2_test.go @@ -13,6 +13,8 @@ import ( ) func TestAccFWPolicyV2_basic(t *testing.T) { + var policy policies.Policy + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -26,7 +28,32 @@ func TestAccFWPolicyV2_basic(t *testing.T) { Config: testAccFWPolicyV2Basic, Check: resource.ComposeTestCheckFunc( testAccCheckFWPolicyV2Exists( - "openstack_fw_policy_v2.policy_1", "", "", 0), + "openstack_fw_policy_v2.policy_1", &policy), + ), + }, + }, + }) +} + +func TestAccFWPolicyV2_shared(t *testing.T) { + var policy policies.Policy + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2Shared, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "shared", "true"), ), }, }, @@ -34,6 +61,8 @@ func TestAccFWPolicyV2_basic(t *testing.T) { } func TestAccFWPolicyV2_addRules(t *testing.T) { + var policy policies.Policy + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -47,7 +76,9 @@ func TestAccFWPolicyV2_addRules(t *testing.T) { Config: testAccFWPolicyV2AddRules, Check: resource.ComposeTestCheckFunc( testAccCheckFWPolicyV2Exists( - "openstack_fw_policy_v2.policy_1", "policy_1", "terraform acceptance test", 2), + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "audited", "true"), ), }, }, @@ -55,6 +86,8 @@ func TestAccFWPolicyV2_addRules(t *testing.T) { } func TestAccFWPolicyV2_deleteRules(t *testing.T) { + var policy policies.Policy + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) @@ -68,7 +101,163 @@ func TestAccFWPolicyV2_deleteRules(t *testing.T) { Config: testAccFWPolicyV2DeleteRules, Check: resource.ComposeTestCheckFunc( testAccCheckFWPolicyV2Exists( - "openstack_fw_policy_v2.policy_1", "policy_1", "terraform acceptance test", 1), + "openstack_fw_policy_v2.policy_1", &policy), + ), + }, + }, + }) +} + +func TestAccFWPolicyV2_rulesOrder(t *testing.T) { + var policy policies.Policy + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2RulesOrderBasic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_3", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.3", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "4"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderRemove, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "3"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderRevert, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_3", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.3", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "4"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderBasic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_3", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.3", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "4"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderShuffle, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.3", + "openstack_fw_rule_v2.rule_3", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "4"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderRemove, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "3"), + ), + }, + + { + Config: testAccFWPolicyV2RulesOrderBasic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.0", + "openstack_fw_rule_v2.rule_1", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.1", + "openstack_fw_rule_v2.rule_2", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.2", + "openstack_fw_rule_v2.rule_3", "id"), + resource.TestCheckResourceAttrPair( + "data.openstack_fw_policy_v2.policy_1", "rules.3", + "openstack_fw_rule_v2.rule_4", "id"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "rules.#", "4"), ), }, }, @@ -96,7 +285,7 @@ func testAccCheckFWPolicyV2Destroy(s *terraform.State) error { return nil } -func testAccCheckFWPolicyV2Exists(n, name, description string, ruleCount int) resource.TestCheckFunc { +func testAccCheckFWPolicyV2Exists(n string, policy *policies.Policy) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -128,33 +317,28 @@ func testAccCheckFWPolicyV2Exists(n, name, description string, ruleCount int) re break } - switch { - case name != found.Name: - err = fmt.Errorf("Expected name <%s>, but found <%s>", name, found.Name) - case description != found.Description: - err = fmt.Errorf("Expected description <%s>, but found <%s>", description, found.Description) - case ruleCount != len(found.Rules): - err = fmt.Errorf("Expected rule count <%d>, but found <%d>", ruleCount, len(found.Rules)) - } - - if err != nil { - return err - } + *policy = *found return nil } } const testAccFWPolicyV2Basic = ` +resource "openstack_fw_policy_v2" "policy_1" {} +` + +const testAccFWPolicyV2Shared = ` resource "openstack_fw_policy_v2" "policy_1" { + shared = true } ` const testAccFWPolicyV2AddRules = ` resource "openstack_fw_policy_v2" "policy_1" { - name = "policy_1" - description = "terraform acceptance test" - rules = [ + name = "policy_1" + description = "terraform acceptance test" + audited = true + rules = [ "${openstack_fw_rule_v2.udp_deny.id}", "${openstack_fw_rule_v2.tcp_allow.id}" ] @@ -162,26 +346,173 @@ resource "openstack_fw_policy_v2" "policy_1" { resource "openstack_fw_rule_v2" "tcp_allow" { protocol = "tcp" - action = "allow" + action = "allow" } resource "openstack_fw_rule_v2" "udp_deny" { protocol = "udp" - action = "deny" + action = "deny" } ` const testAccFWPolicyV2DeleteRules = ` resource "openstack_fw_policy_v2" "policy_1" { - name = "policy_1" - description = "terraform acceptance test" - rules = [ + name = "policy_1" + description = "terraform acceptance test" + rules = [ "${openstack_fw_rule_v2.udp_deny.id}" ] } resource "openstack_fw_rule_v2" "udp_deny" { protocol = "udp" - action = "deny" + action = "deny" +} +` + +const testAccFWPolicyV2RulesOrderBasic = ` +resource "openstack_fw_rule_v2" "rule_1" { + protocol = "tcp" + action = "deny" +} + +resource "openstack_fw_rule_v2" "rule_2" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_3" { + protocol = "udp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_4" { + protocol = "udp" + action = "deny" +} + +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.rule_1.id}", + "${openstack_fw_rule_v2.rule_2.id}", + "${openstack_fw_rule_v2.rule_3.id}", + "${openstack_fw_rule_v2.rule_4.id}" + ] +} + +data "openstack_fw_policy_v2" "policy_1" { + policy_id = "${openstack_fw_policy_v2.policy_1.id}" +} +` + +const testAccFWPolicyV2RulesOrderRemove = ` +resource "openstack_fw_rule_v2" "rule_1" { + protocol = "tcp" + action = "deny" +} + +resource "openstack_fw_rule_v2" "rule_2" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_3" { + protocol = "udp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_4" { + protocol = "udp" + action = "deny" +} + +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.rule_1.id}", + "${openstack_fw_rule_v2.rule_2.id}", + "${openstack_fw_rule_v2.rule_4.id}" + ] +} + +data "openstack_fw_policy_v2" "policy_1" { + policy_id = "${openstack_fw_policy_v2.policy_1.id}" +} +` + +const testAccFWPolicyV2RulesOrderRevert = ` +resource "openstack_fw_rule_v2" "rule_1" { + protocol = "tcp" + action = "deny" +} + +resource "openstack_fw_rule_v2" "rule_2" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_3" { + protocol = "udp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_4" { + protocol = "udp" + action = "deny" +} + +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.rule_4.id}", + "${openstack_fw_rule_v2.rule_3.id}", + "${openstack_fw_rule_v2.rule_2.id}", + "${openstack_fw_rule_v2.rule_1.id}" + ] +} + +data "openstack_fw_policy_v2" "policy_1" { + policy_id = "${openstack_fw_policy_v2.policy_1.id}" +} +` + +const testAccFWPolicyV2RulesOrderShuffle = ` +resource "openstack_fw_rule_v2" "rule_1" { + protocol = "tcp" + action = "deny" +} + +resource "openstack_fw_rule_v2" "rule_2" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_3" { + protocol = "udp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "rule_4" { + protocol = "udp" + action = "deny" +} + +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" + rules = [ + "${openstack_fw_rule_v2.rule_1.id}", + "${openstack_fw_rule_v2.rule_4.id}", + "${openstack_fw_rule_v2.rule_2.id}", + "${openstack_fw_rule_v2.rule_3.id}" + ] +} + +data "openstack_fw_policy_v2" "policy_1" { + policy_id = "${openstack_fw_policy_v2.policy_1.id}" } ` diff --git a/openstack/resource_openstack_fw_rule_v2.go b/openstack/resource_openstack_fw_rule_v2.go index 17ead4e67..f938a9e79 100644 --- a/openstack/resource_openstack_fw_rule_v2.go +++ b/openstack/resource_openstack_fw_rule_v2.go @@ -191,33 +191,43 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in return diag.Errorf("Error creating OpenStack networking client: %s", err) } - var updateOpts rules.UpdateOpts + var ( + hasChange bool + updateOpts rules.UpdateOpts + ) + if d.HasChange("name") { + hasChange = true name := d.Get("name").(string) updateOpts.Name = &name } if d.HasChange("description") { + hasChange = true description := d.Get("description").(string) updateOpts.Description = &description } if d.HasChange("protocol") { + hasChange = true protocol := rules.Protocol(d.Get("protocol").(string)) updateOpts.Protocol = &protocol } if d.HasChange("action") { + hasChange = true action := rules.Action(d.Get("action").(string)) updateOpts.Action = &action } if d.HasChange("ip_version") { + hasChange = true ipVersion := gophercloud.IPVersion(d.Get("ip_version").(int)) updateOpts.IPVersion = &ipVersion } if d.HasChange("source_ip_address") { + hasChange = true sourceIPAddress := d.Get("source_ip_address").(string) updateOpts.SourceIPAddress = &sourceIPAddress @@ -227,6 +237,7 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in } if d.HasChange("source_port") { + hasChange = true sourcePort := d.Get("source_port").(string) if sourcePort == "" { sourcePort = "0" @@ -239,6 +250,7 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in } if d.HasChange("destination_ip_address") { + hasChange = true destinationIPAddress := d.Get("destination_ip_address").(string) updateOpts.DestinationIPAddress = &destinationIPAddress @@ -248,6 +260,7 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in } if d.HasChange("destination_port") { + hasChange = true destinationPort := d.Get("destination_port").(string) if destinationPort == "" { destinationPort = "0" @@ -261,19 +274,24 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in } if d.HasChange("enabled") { + hasChange = true enabled := d.Get("enabled").(bool) updateOpts.Enabled = &enabled } if d.HasChange("shared") { + hasChange = true shared := d.Get("shared").(bool) updateOpts.Enabled = &shared } - log.Printf("[DEBUG] openstack_fw_rule_v2 %s update options: %#v", d.Id(), updateOpts) - err = rules.Update(networkingClient, d.Id(), updateOpts).Err - if err != nil { - return diag.Errorf("Error updating openstack_fw_rule_v2 %s: %s", d.Id(), err) + if hasChange { + log.Printf("[DEBUG] openstack_fw_rule_v2 %s update options: %#v", d.Id(), updateOpts) + + err = rules.Update(networkingClient, d.Id(), updateOpts).Err + if err != nil { + return diag.Errorf("Error updating openstack_fw_rule_v2 %s: %s", d.Id(), err) + } } return resourceFWRuleV2Read(ctx, d, meta) diff --git a/openstack/resource_openstack_fw_rule_v2_test.go b/openstack/resource_openstack_fw_rule_v2_test.go index 3fc6dcc66..5cf3d678b 100644 --- a/openstack/resource_openstack_fw_rule_v2_test.go +++ b/openstack/resource_openstack_fw_rule_v2_test.go @@ -150,6 +150,40 @@ func TestAccFWRuleV2_basic(t *testing.T) { }) } +func TestAccFWRuleV2_shared(t *testing.T) { + var rule rules.Rule + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWRuleV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWRuleV2Shared, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWRuleV2Exists("openstack_fw_rule_v2.rule_1", &rule), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "name", "shared_rule"), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "protocol", "any"), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "action", "deny"), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "ip_version", "4"), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "enabled", "true"), + resource.TestCheckResourceAttr( + "openstack_fw_rule_v2.rule_1", "shared", "true"), + ), + }, + }, + }) +} + func TestAccFWRuleV2_anyProtocol(t *testing.T) { var rule rules.Rule @@ -317,110 +351,117 @@ func testAccCheckFWRuleV2Exists(n string, rule *rules.Rule) resource.TestCheckFu const testAccFWRuleV2Basic1 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - protocol = "udp" - action = "deny" + name = "rule_1" + protocol = "udp" + action = "deny" +} +` + +const testAccFWRuleV2Shared = ` +resource "openstack_fw_rule_v2" "rule_1" { + name = "shared_rule" + shared = true } ` const testAccFWRuleV2Basic2 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Terraform accept test" - protocol = "udp" - action = "deny" - ip_version = 4 - source_ip_address = "1.2.3.4" - destination_ip_address = "4.3.2.0/24" - source_port = "444" - destination_port = "555" - enabled = true - shared = false + name = "rule_1" + description = "Terraform accept test" + protocol = "udp" + action = "deny" + ip_version = 4 + source_ip_address = "1.2.3.4" + destination_ip_address = "4.3.2.0/24" + source_port = "444" + destination_port = "555" + enabled = true + shared = false } ` const testAccFWRuleV2Basic3 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Terraform accept test updated" - protocol = "tcp" - action = "allow" - ip_version = 4 - source_ip_address = "1.2.3.0/24" - destination_ip_address = "4.3.2.8" - source_port = "666" - destination_port = "777" - enabled = false + name = "rule_1" + description = "Terraform accept test updated" + protocol = "tcp" + action = "allow" + ip_version = 4 + source_ip_address = "1.2.3.0/24" + destination_ip_address = "4.3.2.8" + source_port = "666" + destination_port = "777" + enabled = false } ` const testAccFWRuleV2Basic4 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Terraform accept test updated" - protocol = "udp" - action = "allow" - ip_version = 4 - source_ip_address = "1.2.3.0/24" - destination_ip_address = "4.3.2.8" - source_port = "666" - destination_port = "777" - enabled = false + name = "rule_1" + description = "Terraform accept test updated" + protocol = "udp" + action = "allow" + ip_version = 4 + source_ip_address = "1.2.3.0/24" + destination_ip_address = "4.3.2.8" + source_port = "666" + destination_port = "777" + enabled = false } ` const testAccFWRuleV2Basic5 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Terraform accept test updated" - protocol = "udp" - action = "allow" - ip_version = 4 - source_ip_address = "1.2.3.0/24" - destination_ip_address = "4.3.2.8" - source_port = "666" - enabled = false + name = "rule_1" + description = "Terraform accept test updated" + protocol = "udp" + action = "allow" + ip_version = 4 + source_ip_address = "1.2.3.0/24" + destination_ip_address = "4.3.2.8" + source_port = "666" + enabled = false } ` const testAccFWRuleV2AnyProtocol = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Allow any protocol" - protocol = "any" - action = "allow" - ip_version = 4 - source_ip_address = "192.168.199.0/24" - enabled = true + name = "rule_1" + description = "Allow any protocol" + protocol = "any" + action = "allow" + ip_version = 4 + source_ip_address = "192.168.199.0/24" + enabled = true } ` const testAccFWRuleV2UpdateName1 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "rule_1" - description = "Terraform accept test" - protocol = "udp" - action = "deny" - ip_version = 4 - source_ip_address = "1.2.3.4" - destination_ip_address = "4.3.2.0/24" - source_port = "444" - destination_port = "555" - enabled = true + name = "rule_1" + description = "Terraform accept test" + protocol = "udp" + action = "deny" + ip_version = 4 + source_ip_address = "1.2.3.4" + destination_ip_address = "4.3.2.0/24" + source_port = "444" + destination_port = "555" + enabled = true } ` const testAccFWRuleV2UpdateName2 = ` resource "openstack_fw_rule_v2" "rule_1" { - name = "updated_rule_1" - description = "Terraform accept test" - protocol = "udp" - action = "deny" - ip_version = 4 - source_ip_address = "1.2.3.4" - destination_ip_address = "4.3.2.0/24" - source_port = "444" - destination_port = "555" - enabled = true + name = "updated_rule_1" + description = "Terraform accept test" + protocol = "udp" + action = "deny" + ip_version = 4 + source_ip_address = "1.2.3.4" + destination_ip_address = "4.3.2.0/24" + source_port = "444" + destination_port = "555" + enabled = true } ` From 32d338e7429959104b9e014550ec52a19c49e1a5 Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Wed, 21 Jun 2023 11:36:24 +0300 Subject: [PATCH 3/7] Add missing update opts --- openstack/resource_openstack_fw_policy_v2.go | 12 ++++++++++++ openstack/resource_openstack_fw_rule_v2.go | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/openstack/resource_openstack_fw_policy_v2.go b/openstack/resource_openstack_fw_policy_v2.go index 5ff17d914..ff25eee4f 100644 --- a/openstack/resource_openstack_fw_policy_v2.go +++ b/openstack/resource_openstack_fw_policy_v2.go @@ -162,6 +162,18 @@ func resourceFWPolicyV2Update(ctx context.Context, d *schema.ResourceData, meta updateOpts.FirewallRules = &rules } + if d.HasChange("audited") { + hasChange = true + audited := d.Get("audited").(bool) + updateOpts.Audited = &audited + } + + if d.HasChange("shared") { + hasChange = true + shared := d.Get("shared").(bool) + updateOpts.Shared = &shared + } + if hasChange { log.Printf("[DEBUG] openstack_fw_policy_v2 %s update options: %#v", d.Id(), updateOpts) diff --git a/openstack/resource_openstack_fw_rule_v2.go b/openstack/resource_openstack_fw_rule_v2.go index f938a9e79..87f244f87 100644 --- a/openstack/resource_openstack_fw_rule_v2.go +++ b/openstack/resource_openstack_fw_rule_v2.go @@ -282,7 +282,7 @@ func resourceFWRuleV2Update(ctx context.Context, d *schema.ResourceData, meta in if d.HasChange("shared") { hasChange = true shared := d.Get("shared").(bool) - updateOpts.Enabled = &shared + updateOpts.Shared = &shared } if hasChange { From b8b5a12297fba2b4239b4d8d945e84a6e086c5ce Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Wed, 21 Jun 2023 12:34:02 +0300 Subject: [PATCH 4/7] Add ErrDefault404 to policy deletion, fix datasource panic, review --- .../data_source_openstack_fw_policy_v2.go | 6 ++- ...data_source_openstack_fw_policy_v2_test.go | 47 +++++++++++++++++++ openstack/data_source_openstack_fw_rule_v2.go | 6 ++- openstack/fw_policy_v2.go | 19 +++++--- openstack/resource_openstack_fw_policy_v2.go | 11 +++-- website/docs/r/fw_policy_v2.html.markdown | 3 -- 6 files changed, 75 insertions(+), 17 deletions(-) diff --git a/openstack/data_source_openstack_fw_policy_v2.go b/openstack/data_source_openstack_fw_policy_v2.go index b2e35fe37..ffd93de0f 100644 --- a/openstack/data_source_openstack_fw_policy_v2.go +++ b/openstack/data_source_openstack_fw_policy_v2.go @@ -91,11 +91,13 @@ func dataSourceFWPolicyV2Read(ctx context.Context, d *schema.ResourceData, meta } if v, ok := d.GetOk("shared"); ok { - listOpts.Shared = v.(*bool) + shared := v.(bool) + listOpts.Shared = &shared } if v, ok := d.GetOk("audited"); ok { - listOpts.Audited = v.(*bool) + audited := v.(bool) + listOpts.Audited = &audited } pages, err := policies.List(networkingClient, listOpts).AllPages() diff --git a/openstack/data_source_openstack_fw_policy_v2_test.go b/openstack/data_source_openstack_fw_policy_v2_test.go index f8a2e8a87..caad3b7fe 100644 --- a/openstack/data_source_openstack_fw_policy_v2_test.go +++ b/openstack/data_source_openstack_fw_policy_v2_test.go @@ -39,6 +39,36 @@ func TestAccFWPolicyV2DataSource_basic(t *testing.T) { }, }) } + +func TestAccFWPolicyV2DataSource_shared(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2DataSourceShared, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2DataSourceID("data.openstack_fw_policy_v2.policy_1"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "name", "policy_1"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "description", "My firewall policy"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "shared", "true"), + resource.TestCheckResourceAttr( + "data.openstack_fw_policy_v2.policy_1", "audited", "true"), + resource.TestCheckResourceAttrSet( + "data.openstack_fw_policy_v2.policy_1", "tenant_id"), + ), + }, + }, + }) +} + func TestAccFWPolicyV2DataSource_FWPolicyID(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { @@ -85,6 +115,23 @@ resource "openstack_fw_policy_v2" "policy_1" { } ` +const testAccFWPolicyV2DataSourceShared = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "My firewall policy" + shared = true + audited = true +} + +data "openstack_fw_policy_v2" "policy_1" { + name = "${openstack_fw_policy_v2.policy_1.name}" + description = "${openstack_fw_policy_v2.policy_1.description}" + tenant_id = "${openstack_fw_policy_v2.policy_1.tenant_id}" + shared = true + audited = true +} +` + func testAccFWPolicyV2DataSourceName() string { return fmt.Sprintf(` %s diff --git a/openstack/data_source_openstack_fw_rule_v2.go b/openstack/data_source_openstack_fw_rule_v2.go index d67266db1..5ec5da194 100644 --- a/openstack/data_source_openstack_fw_rule_v2.go +++ b/openstack/data_source_openstack_fw_rule_v2.go @@ -166,11 +166,13 @@ func dataSourceFWRuleV2Read(ctx context.Context, d *schema.ResourceData, meta in } if v, ok := d.GetOk("shared"); ok { - listOpts.Shared = v.(*bool) + shared := v.(bool) + listOpts.Shared = &shared } if v, ok := d.GetOk("enabled"); ok { - listOpts.Enabled = v.(*bool) + enabled := v.(bool) + listOpts.Enabled = &enabled } pages, err := rules.List(networkingClient, listOpts).AllPages() diff --git a/openstack/fw_policy_v2.go b/openstack/fw_policy_v2.go index 9a3cca667..267e383ad 100644 --- a/openstack/fw_policy_v2.go +++ b/openstack/fw_policy_v2.go @@ -1,6 +1,8 @@ package openstack import ( + "log" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/gophercloud/gophercloud" @@ -14,12 +16,17 @@ func fwPolicyV2DeleteFunc(networkingClient *gophercloud.ServiceClient, id string return "", "DELETED", nil } - if _, ok := err.(gophercloud.ErrDefault409); ok { - // This error usually means that the policy is attached - // to a firewall. At this point, the firewall is probably - // being delete. So, we retry a few times. - - return nil, "ACTIVE", nil + if err != nil { + switch err.(type) { + case gophercloud.ErrDefault404: + // This error usually means that the policy was deleted manually + log.Printf("[DEBUG] Unable to find openstack_fw_policy_v2 %s: %s", id, err) + return "", "DELETED", nil + case gophercloud.ErrDefault409: + // This error usually means that the policy is attached to a firewall. + log.Printf("[DEBUG] Error to delete openstack_fw_policy_v2 %s: %s", id, err) + return nil, "ACTIVE", nil + } } return nil, "ACTIVE", err diff --git a/openstack/resource_openstack_fw_policy_v2.go b/openstack/resource_openstack_fw_policy_v2.go index ff25eee4f..5595003ce 100644 --- a/openstack/resource_openstack_fw_policy_v2.go +++ b/openstack/resource_openstack_fw_policy_v2.go @@ -79,17 +79,20 @@ func resourceFWPolicyV2Create(ctx context.Context, d *schema.ResourceData, meta return diag.Errorf("Error creating OpenStack networking client: %s", err) } - audited := d.Get("audited").(bool) opts := policies.CreateOpts{ Name: d.Get("name").(string), Description: d.Get("description").(string), - Audited: &audited, TenantID: d.Get("tenant_id").(string), FirewallRules: expandToStringSlice(d.Get("rules").([]interface{})), } - if r, ok := d.GetOk("shared"); ok { - shared := r.(bool) + if v, ok := d.GetOk("audited"); ok { + audited := v.(bool) + opts.Audited = &audited + } + + if v, ok := d.GetOk("shared"); ok { + shared := v.(bool) opts.Shared = &shared } diff --git a/website/docs/r/fw_policy_v2.html.markdown b/website/docs/r/fw_policy_v2.html.markdown index 20d03c375..b2f5f3999 100644 --- a/website/docs/r/fw_policy_v2.html.markdown +++ b/website/docs/r/fw_policy_v2.html.markdown @@ -79,9 +79,6 @@ The following arguments are supported: `shared` status of an existing firewall policy. Only administrative users can specify if the policy should be shared. -* `value_specs` - (Optional) Map of additional options. Changing this creates a - new firewall policy. - ## Attributes Reference The following attributes are exported: From dad8d852612419027866b28ea90992be34505ae1 Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Wed, 21 Jun 2023 15:41:14 +0300 Subject: [PATCH 5/7] Add missing step to policy datasource test --- openstack/data_source_openstack_fw_policy_v2_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openstack/data_source_openstack_fw_policy_v2_test.go b/openstack/data_source_openstack_fw_policy_v2_test.go index caad3b7fe..cccc178d4 100644 --- a/openstack/data_source_openstack_fw_policy_v2_test.go +++ b/openstack/data_source_openstack_fw_policy_v2_test.go @@ -49,6 +49,9 @@ func TestAccFWPolicyV2DataSource_shared(t *testing.T) { }, ProviderFactories: testAccProviders, Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2DataSourceBasic, + }, { Config: testAccFWPolicyV2DataSourceShared, Check: resource.ComposeTestCheckFunc( From 554328a557049269d9d9dc37a6b54e6152e0947f Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Wed, 21 Jun 2023 17:56:54 +0300 Subject: [PATCH 6/7] Review policy tests, revert policies.RemoveRule to rule deletion --- .../resource_openstack_fw_policy_v2_test.go | 126 +++++++++++++++--- openstack/resource_openstack_fw_rule_v2.go | 16 +++ 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/openstack/resource_openstack_fw_policy_v2_test.go b/openstack/resource_openstack_fw_policy_v2_test.go index 67ae6bf88..3c1ef4639 100644 --- a/openstack/resource_openstack_fw_policy_v2_test.go +++ b/openstack/resource_openstack_fw_policy_v2_test.go @@ -72,13 +72,71 @@ func TestAccFWPolicyV2_addRules(t *testing.T) { ProviderFactories: testAccProviders, CheckDestroy: testAccCheckFWPolicyV2Destroy, Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2Basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "rules.#", "0"), + ), + }, { Config: testAccFWPolicyV2AddRules, Check: resource.ComposeTestCheckFunc( testAccCheckFWPolicyV2Exists( "openstack_fw_policy_v2.policy_1", &policy), resource.TestCheckResourceAttr( - "openstack_fw_policy_v2.policy_1", "audited", "true"), + "openstack_fw_policy_v2.policy_1", "rules.#", "2"), + ), + }, + }, + }) +} + +func TestAccFWPolicyV2_clearFields(t *testing.T) { + var policy policies.Policy + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckNonAdminOnly(t) + testAccPreCheckFW(t) + }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckFWPolicyV2Destroy, + Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2Basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "name", ""), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "description", ""), + ), + }, + { + Config: testAccFWPolicyV2FillOutFields, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "name", "policy_1"), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "description", "terraform acceptance test"), + ), + }, + { + Config: testAccFWPolicyV2ClearFields, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "name", ""), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "description", ""), ), }, }, @@ -97,11 +155,31 @@ func TestAccFWPolicyV2_deleteRules(t *testing.T) { ProviderFactories: testAccProviders, CheckDestroy: testAccCheckFWPolicyV2Destroy, Steps: []resource.TestStep{ + { + Config: testAccFWPolicyV2AddRules, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "rules.#", "2"), + ), + }, { Config: testAccFWPolicyV2DeleteRules, Check: resource.ComposeTestCheckFunc( testAccCheckFWPolicyV2Exists( "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "rules.#", "1"), + ), + }, + { + Config: testAccFWPolicyV2Basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckFWPolicyV2Exists( + "openstack_fw_policy_v2.policy_1", &policy), + resource.TestCheckResourceAttr( + "openstack_fw_policy_v2.policy_1", "rules.#", "0"), ), }, }, @@ -147,13 +225,13 @@ func TestAccFWPolicyV2_rulesOrder(t *testing.T) { testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.0", - "openstack_fw_rule_v2.rule_1", "id"), + "openstack_fw_rule_v2.rule_4", "id"), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.1", "openstack_fw_rule_v2.rule_2", "id"), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.2", - "openstack_fw_rule_v2.rule_4", "id"), + "openstack_fw_rule_v2.rule_1", "id"), resource.TestCheckResourceAttr( "data.openstack_fw_policy_v2.policy_1", "rules.#", "3"), ), @@ -228,13 +306,13 @@ func TestAccFWPolicyV2_rulesOrder(t *testing.T) { testAccCheckFWPolicyV2Exists("openstack_fw_policy_v2.policy_1", &policy), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.0", - "openstack_fw_rule_v2.rule_1", "id"), + "openstack_fw_rule_v2.rule_4", "id"), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.1", "openstack_fw_rule_v2.rule_2", "id"), resource.TestCheckResourceAttrPair( "data.openstack_fw_policy_v2.policy_1", "rules.2", - "openstack_fw_rule_v2.rule_4", "id"), + "openstack_fw_rule_v2.rule_1", "id"), resource.TestCheckResourceAttr( "data.openstack_fw_policy_v2.policy_1", "rules.#", "3"), ), @@ -334,6 +412,16 @@ resource "openstack_fw_policy_v2" "policy_1" { ` const testAccFWPolicyV2AddRules = ` +resource "openstack_fw_rule_v2" "tcp_allow" { + protocol = "tcp" + action = "allow" +} + +resource "openstack_fw_rule_v2" "udp_deny" { + protocol = "udp" + action = "deny" +} + resource "openstack_fw_policy_v2" "policy_1" { name = "policy_1" description = "terraform acceptance test" @@ -343,19 +431,28 @@ resource "openstack_fw_policy_v2" "policy_1" { "${openstack_fw_rule_v2.tcp_allow.id}" ] } +` -resource "openstack_fw_rule_v2" "tcp_allow" { - protocol = "tcp" - action = "allow" +const testAccFWPolicyV2FillOutFields = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "policy_1" + description = "terraform acceptance test" +} +` + +const testAccFWPolicyV2ClearFields = ` +resource "openstack_fw_policy_v2" "policy_1" { + name = "" + description = "" } +` +const testAccFWPolicyV2DeleteRules = ` resource "openstack_fw_rule_v2" "udp_deny" { protocol = "udp" action = "deny" } -` -const testAccFWPolicyV2DeleteRules = ` resource "openstack_fw_policy_v2" "policy_1" { name = "policy_1" description = "terraform acceptance test" @@ -363,11 +460,6 @@ resource "openstack_fw_policy_v2" "policy_1" { "${openstack_fw_rule_v2.udp_deny.id}" ] } - -resource "openstack_fw_rule_v2" "udp_deny" { - protocol = "udp" - action = "deny" -} ` const testAccFWPolicyV2RulesOrderBasic = ` @@ -432,9 +524,9 @@ resource "openstack_fw_policy_v2" "policy_1" { name = "policy_1" description = "terraform acceptance test" rules = [ - "${openstack_fw_rule_v2.rule_1.id}", + "${openstack_fw_rule_v2.rule_4.id}", "${openstack_fw_rule_v2.rule_2.id}", - "${openstack_fw_rule_v2.rule_4.id}" + "${openstack_fw_rule_v2.rule_1.id}" ] } diff --git a/openstack/resource_openstack_fw_rule_v2.go b/openstack/resource_openstack_fw_rule_v2.go index 87f244f87..6bb80d1d8 100644 --- a/openstack/resource_openstack_fw_rule_v2.go +++ b/openstack/resource_openstack_fw_rule_v2.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/policies" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/fwaas_v2/rules" ) @@ -304,6 +305,21 @@ func resourceFWRuleV2Delete(_ context.Context, d *schema.ResourceData, meta inte return diag.Errorf("Error creating OpenStack networking client: %s", err) } + rule, err := rules.Get(networkingClient, d.Id()).Extract() + if err != nil { + return diag.FromErr(CheckDeleted(d, err, "Error retrieving openstack_fw_rule_v2")) + } + + if len(rule.FirewallPolicyID) > 0 { + for _, firewallPolicyID := range rule.FirewallPolicyID { + log.Printf("[DEBUG] openstack_fw_rule_v2 %s associate with openstack_fw_policy_v2: %#v", d.Id(), firewallPolicyID) + _, err := policies.RemoveRule(networkingClient, firewallPolicyID, rule.ID).Extract() + if err != nil { + return diag.Errorf("Error removing openstack_fw_rule_v2 %s from policy %s: %s", d.Id(), firewallPolicyID, err) + } + } + } + err = rules.Delete(networkingClient, d.Id()).ExtractErr() if err != nil { return diag.Errorf("Error deleting openstack_fw_rule_v2 %s: %s", d.Id(), err) From 7a2917c6eb27de4a8af9e5c0160f544c446dad16 Mon Sep 17 00:00:00 2001 From: Konstantin Eremin Date: Thu, 22 Jun 2023 12:46:43 +0300 Subject: [PATCH 7/7] Small review --- openstack/data_source_openstack_fw_policy_v2.go | 2 +- openstack/data_source_openstack_fw_rule_v2.go | 2 +- openstack/resource_openstack_fw_policy_v2.go | 1 - openstack/resource_openstack_fw_rule_v2.go | 15 ++++++++++----- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/openstack/data_source_openstack_fw_policy_v2.go b/openstack/data_source_openstack_fw_policy_v2.go index ffd93de0f..ba4383e7a 100644 --- a/openstack/data_source_openstack_fw_policy_v2.go +++ b/openstack/data_source_openstack_fw_policy_v2.go @@ -102,7 +102,7 @@ func dataSourceFWPolicyV2Read(ctx context.Context, d *schema.ResourceData, meta pages, err := policies.List(networkingClient, listOpts).AllPages() if err != nil { - return diag.FromErr(err) + return diag.Errorf("Unable to list openstack_fw_policy_v2 policies: %s", err) } allFWPolicies, err := policies.ExtractPolicies(pages) diff --git a/openstack/data_source_openstack_fw_rule_v2.go b/openstack/data_source_openstack_fw_rule_v2.go index 5ec5da194..92647bf08 100644 --- a/openstack/data_source_openstack_fw_rule_v2.go +++ b/openstack/data_source_openstack_fw_rule_v2.go @@ -177,7 +177,7 @@ func dataSourceFWRuleV2Read(ctx context.Context, d *schema.ResourceData, meta in pages, err := rules.List(networkingClient, listOpts).AllPages() if err != nil { - return diag.FromErr(err) + return diag.Errorf("Unable to list openstack_fw_rule_v2 rules: %s", err) } allFWRules, err := rules.ExtractRules(pages) diff --git a/openstack/resource_openstack_fw_policy_v2.go b/openstack/resource_openstack_fw_policy_v2.go index 5595003ce..9e7690abd 100644 --- a/openstack/resource_openstack_fw_policy_v2.go +++ b/openstack/resource_openstack_fw_policy_v2.go @@ -55,7 +55,6 @@ func resourceFWPolicyV2() *schema.Resource { "audited": { Type: schema.TypeBool, Optional: true, - Default: false, }, "shared": { diff --git a/openstack/resource_openstack_fw_rule_v2.go b/openstack/resource_openstack_fw_rule_v2.go index 6bb80d1d8..ae65e3b74 100644 --- a/openstack/resource_openstack_fw_rule_v2.go +++ b/openstack/resource_openstack_fw_rule_v2.go @@ -99,7 +99,6 @@ func resourceFWRuleV2() *schema.Resource { "shared": { Type: schema.TypeBool, Optional: true, - Default: false, }, "enabled": { @@ -118,8 +117,6 @@ func resourceFWRuleV2Create(ctx context.Context, d *schema.ResourceData, meta in return diag.Errorf("Error creating OpenStack networking client: %s", err) } - shared := d.Get("shared").(bool) - enabled := d.Get("enabled").(bool) ruleCreateOpts := rules.CreateOpts{ Name: d.Get("name").(string), Description: d.Get("description").(string), @@ -130,11 +127,19 @@ func resourceFWRuleV2Create(ctx context.Context, d *schema.ResourceData, meta in DestinationIPAddress: d.Get("destination_ip_address").(string), SourcePort: d.Get("source_port").(string), DestinationPort: d.Get("destination_port").(string), - Shared: &shared, - Enabled: &enabled, TenantID: d.Get("tenant_id").(string), } + if v, ok := d.GetOk("shared"); ok { + shared := v.(bool) + ruleCreateOpts.Shared = &shared + } + + if v, ok := d.GetOk("enabled"); ok { + enabled := v.(bool) + ruleCreateOpts.Enabled = &enabled + } + log.Printf("[DEBUG] openstack_fw_rule_v2 create options: %#v", ruleCreateOpts) rule, err := rules.Create(networkingClient, ruleCreateOpts).Extract()