From 61782bfdc68089b001695c9088c585361c285c2d Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Mon, 1 Jul 2024 08:17:20 +0000 Subject: [PATCH 1/5] correctly populate protocol --- .../services/paloalto/local_rulestack_rule_resource.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/services/paloalto/local_rulestack_rule_resource.go b/internal/services/paloalto/local_rulestack_rule_resource.go index 797f642d1ea9..b5615cfffee7 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource.go +++ b/internal/services/paloalto/local_rulestack_rule_resource.go @@ -152,6 +152,10 @@ func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { Default: protocolApplicationDefault, ValidateFunc: validate.ProtocolWithPort, ConflictsWith: []string{"protocol_ports"}, + // if `protocol_ports` is set, the default value should not be used + DiffSuppressFunc: func(k, old, new string, d *pluginsdk.ResourceData) bool { + return len(d.Get("protocol_ports").([]interface{})) > 0 && old == "" && new == protocolApplicationDefault + }, }, "protocol_ports": { @@ -341,11 +345,7 @@ func (r LocalRuleStackRule) Read() sdk.ResourceFunc { } state.NegateDestination = boolEnumAsBoolRule(props.NegateDestination) state.NegateSource = boolEnumAsBoolRule(props.NegateSource) - if v := pointer.From(props.Protocol); v != "" && !strings.EqualFold(v, protocolApplicationDefault) { - state.Protocol = v - } else { - state.Protocol = protocolApplicationDefault - } + state.Protocol = pointer.From(props.Protocol) state.ProtocolPorts = pointer.From(props.ProtocolPortList) state.RuleEnabled = stateEnumAsBool(props.RuleState) state.Source = schema.FlattenSource(props.Source, *id) From b56bed8b1bac1b0b12b8eb2ada8d4913bed29fea Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Wed, 3 Jul 2024 15:07:05 +0000 Subject: [PATCH 2/5] check protocol value in test --- .../paloalto/local_rulestack_rule_resource_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/services/paloalto/local_rulestack_rule_resource_test.go b/internal/services/paloalto/local_rulestack_rule_resource_test.go index 6352bf0c623c..f23757e28a83 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource_test.go +++ b/internal/services/paloalto/local_rulestack_rule_resource_test.go @@ -93,6 +93,7 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -100,6 +101,7 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.complete(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -107,6 +109,7 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.completeUpdate(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").IsEmpty(), ), }, data.ImportStep(), @@ -114,6 +117,7 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -130,6 +134,7 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -137,6 +142,7 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocol(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -144,6 +150,7 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocolPorts(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").IsEmpty(), ), }, data.ImportStep(), @@ -151,6 +158,7 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocol(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -158,6 +166,7 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), From 634a798651eaf1e6a67d5c43e77c10f82e51437f Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Thu, 4 Jul 2024 09:39:16 +0000 Subject: [PATCH 3/5] remove duplicate check --- .../paloalto/local_rulestack_rule_resource_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/services/paloalto/local_rulestack_rule_resource_test.go b/internal/services/paloalto/local_rulestack_rule_resource_test.go index f23757e28a83..6352bf0c623c 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource_test.go +++ b/internal/services/paloalto/local_rulestack_rule_resource_test.go @@ -93,7 +93,6 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -101,7 +100,6 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.complete(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -109,7 +107,6 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.completeUpdate(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").IsEmpty(), ), }, data.ImportStep(), @@ -117,7 +114,6 @@ func TestAccPaloAltoLocalRule_update(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -134,7 +130,6 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), @@ -142,7 +137,6 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocol(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -150,7 +144,6 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocolPorts(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").IsEmpty(), ), }, data.ImportStep(), @@ -158,7 +151,6 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basicProtocol(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("TCP:8080"), ), }, data.ImportStep(), @@ -166,7 +158,6 @@ func TestAccPaloAltoLocalRule_updateProtocol(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("protocol").HasValue("application-default"), ), }, data.ImportStep(), From 29d038afe4b7bff444e95421a7f9f8e547a21095 Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Thu, 4 Jul 2024 13:28:39 +0000 Subject: [PATCH 4/5] remove protocol default value in 4.0 schema --- .../paloalto/local_rulestack_rule_resource.go | 53 ++++++++---- .../local_rulestack_rule_resource_test.go | 80 ++++++++++++++++++- 2 files changed, 118 insertions(+), 15 deletions(-) diff --git a/internal/services/paloalto/local_rulestack_rule_resource.go b/internal/services/paloalto/local_rulestack_rule_resource.go index b5615cfffee7..3f79a18a7e9e 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource.go +++ b/internal/services/paloalto/local_rulestack_rule_resource.go @@ -16,6 +16,7 @@ import ( certificates "github.com/hashicorp/go-azure-sdk/resource-manager/paloaltonetworks/2022-08-29/certificateobjectlocalrulestack" "github.com/hashicorp/go-azure-sdk/resource-manager/paloaltonetworks/2022-08-29/localrules" "github.com/hashicorp/go-azure-sdk/resource-manager/paloaltonetworks/2022-08-29/localrulestacks" + "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/locks" "github.com/hashicorp/terraform-provider-azurerm/internal/sdk" "github.com/hashicorp/terraform-provider-azurerm/internal/services/paloalto/schema" @@ -62,7 +63,7 @@ func (r LocalRuleStackRule) ResourceType() string { } func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { - return map[string]*pluginsdk.Schema{ + schema := map[string]*pluginsdk.Schema{ "name": { Type: pluginsdk.TypeString, Required: true, @@ -146,7 +147,19 @@ func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { Default: false, }, - "protocol": { + "enabled": { + Type: pluginsdk.TypeBool, + Optional: true, + Default: true, + }, + + "source": schema.SourceSchema(), + + "tags": commonschema.Tags(), + } + + if !features.FourPointOhBeta() { + schema["protocol"] = &pluginsdk.Schema{ Type: pluginsdk.TypeString, Optional: true, Default: protocolApplicationDefault, @@ -156,9 +169,9 @@ func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { DiffSuppressFunc: func(k, old, new string, d *pluginsdk.ResourceData) bool { return len(d.Get("protocol_ports").([]interface{})) > 0 && old == "" && new == protocolApplicationDefault }, - }, + } - "protocol_ports": { + schema["protocol_ports"] = &pluginsdk.Schema{ Type: pluginsdk.TypeList, Optional: true, MinItems: 1, @@ -167,18 +180,30 @@ func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { ValidateFunc: validate.ProtocolWithPort, }, ConflictsWith: []string{"protocol"}, - }, - - "enabled": { - Type: pluginsdk.TypeBool, + } + } else { + schema["protocol"] = &pluginsdk.Schema{ + Type: pluginsdk.TypeString, Optional: true, - Default: true, - }, - - "source": schema.SourceSchema(), - - "tags": commonschema.Tags(), + ValidateFunc: validation.Any( + validate.ProtocolWithPort, + validation.StringInSlice([]string{protocolApplicationDefault}, false), + ), + ExactlyOneOf: []string{"protocol", "protocol_ports"}, + } + + schema["protocol_ports"] = &pluginsdk.Schema{ + Type: pluginsdk.TypeList, + Optional: true, + MinItems: 1, + Elem: &pluginsdk.Schema{ + Type: pluginsdk.TypeString, + ValidateFunc: validate.ProtocolWithPort, + }, + ExactlyOneOf: []string{"protocol", "protocol_ports"}, + } } + return schema } func (r LocalRuleStackRule) Attributes() map[string]*pluginsdk.Schema { diff --git a/internal/services/paloalto/local_rulestack_rule_resource_test.go b/internal/services/paloalto/local_rulestack_rule_resource_test.go index 6352bf0c623c..92b9ae99608e 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource_test.go +++ b/internal/services/paloalto/local_rulestack_rule_resource_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" ) @@ -182,6 +183,34 @@ func (r LocalRuleResource) Exists(ctx context.Context, client *clients.Client, s } func (r LocalRuleResource) basic(data acceptance.TestData) string { + if features.FourPointOhBeta() { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%[1]s + +resource "azurerm_palo_alto_local_rulestack_rule" "test" { + name = "testacc-palr-%[2]d" + rulestack_id = azurerm_palo_alto_local_rulestack.test.id + priority = 100 + action = "Allow" + protocol = "application-default" + + applications = ["any"] + + destination { + cidrs = ["any"] + } + + source { + cidrs = ["any"] + } +} +`, r.template(data), data.RandomInteger) + } + return fmt.Sprintf(` provider "azurerm" { features {} @@ -267,9 +296,30 @@ resource "azurerm_palo_alto_local_rulestack_rule" "test" { } func (r LocalRuleResource) requiresImport(data acceptance.TestData) string { - return fmt.Sprintf(` + if features.FourPointOhBeta() { + return fmt.Sprintf(` +%[1]s + +resource "azurerm_palo_alto_local_rulestack_rule" "import" { + name = azurerm_palo_alto_local_rulestack_rule.test.name + rulestack_id = azurerm_palo_alto_local_rulestack_rule.test.rulestack_id + priority = azurerm_palo_alto_local_rulestack_rule.test.priority + action = "Allow" + applications = azurerm_palo_alto_local_rulestack_rule.test.applications + protocol = azurerm_palo_alto_local_rulestack_rule.test.protocol + + destination { + cidrs = azurerm_palo_alto_local_rulestack_rule.test.destination.0.cidrs + } + source { + cidrs = azurerm_palo_alto_local_rulestack_rule.test.source.0.cidrs + } +} +`, r.basic(data), data.RandomInteger) + } + return fmt.Sprintf(` %[1]s resource "azurerm_palo_alto_local_rulestack_rule" "import" { @@ -291,6 +341,34 @@ resource "azurerm_palo_alto_local_rulestack_rule" "import" { } func (r LocalRuleResource) withDestination(data acceptance.TestData) string { + if features.FourPointOhBeta() { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%[1]s + +resource "azurerm_palo_alto_local_rulestack_rule" "test" { + name = "testacc-palr-%[2]d" + rulestack_id = azurerm_palo_alto_local_rulestack.test.id + priority = 100 + action = "Allow" + protocol = "application-default" + + applications = ["any"] + + destination { + countries = ["US", "GB"] + } + + source { + countries = ["US", "GB"] + } +} +`, r.template(data), data.RandomInteger) + } + return fmt.Sprintf(` provider "azurerm" { features {} From 775028d343bf73754a8708c7fd357d97161b2fca Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:12:47 +0000 Subject: [PATCH 5/5] allow protocol=application-default in 3.0 version; add note in doc --- .../paloalto/local_rulestack_rule_resource.go | 11 +++++++---- .../r/palo_alto_local_rulestack_rule.html.markdown | 3 +++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/services/paloalto/local_rulestack_rule_resource.go b/internal/services/paloalto/local_rulestack_rule_resource.go index 3f79a18a7e9e..445b282b2e1c 100644 --- a/internal/services/paloalto/local_rulestack_rule_resource.go +++ b/internal/services/paloalto/local_rulestack_rule_resource.go @@ -160,10 +160,13 @@ func (r LocalRuleStackRule) Arguments() map[string]*pluginsdk.Schema { if !features.FourPointOhBeta() { schema["protocol"] = &pluginsdk.Schema{ - Type: pluginsdk.TypeString, - Optional: true, - Default: protocolApplicationDefault, - ValidateFunc: validate.ProtocolWithPort, + Type: pluginsdk.TypeString, + Optional: true, + Default: protocolApplicationDefault, + ValidateFunc: validation.Any( + validate.ProtocolWithPort, + validation.StringInSlice([]string{protocolApplicationDefault}, false), + ), ConflictsWith: []string{"protocol_ports"}, // if `protocol_ports` is set, the default value should not be used DiffSuppressFunc: func(k, old, new string, d *pluginsdk.ResourceData) bool { diff --git a/website/docs/r/palo_alto_local_rulestack_rule.html.markdown b/website/docs/r/palo_alto_local_rulestack_rule.html.markdown index 5369efa52edf..d381be9069a1 100644 --- a/website/docs/r/palo_alto_local_rulestack_rule.html.markdown +++ b/website/docs/r/palo_alto_local_rulestack_rule.html.markdown @@ -29,6 +29,7 @@ resource "azurerm_palo_alto_local_rulestack_rule" "example" { rulestack_id = azurerm_palo_alto_local_rulestack.example.id priority = 1000 action = "Allow" + protocol = "application-default" applications = ["any"] @@ -85,6 +86,8 @@ The following arguments are supported: * `protocol` - (Optional) The Protocol and port to use in the form `[protocol]:[port_number]` e.g. `TCP:8080` or `UDP:53`. Conflicts with `protocol_ports`. Defaults to `application-default`. +~> **NOTE**: In 4.0 or later versions of the provider, the default of `protocol` will no longer be set by provider, exactly one of `protocol` and `protocol_ports` must be specified. You need to explicitly specify `protocol="application-default"` to keep the the current default of the `protocol`. + * `protocol_ports` - (Optional) Specifies a list of Protocol:Port entries. E.g. `[ "TCP:80", "UDP:5431" ]`. Conflicts with `protocol`. * `tags` - (Optional) A mapping of tags which should be assigned to the Palo Alto Local Rulestack Rule.