From b4e2b870d4f7855af120040b4a9518a732461bb6 Mon Sep 17 00:00:00 2001 From: Krzysztof Klimonda Date: Wed, 22 Jan 2025 18:42:32 +0100 Subject: [PATCH 1/2] Fix handling of empty plural resources --- assets/terraform/internal/manager/entry.go | 2 +- assets/terraform/internal/manager/uuid.go | 4 +-- .../terraform/test/resource_addresses_test.go | 28 ++++++++++++++++ .../test/resource_security_policy_test.go | 32 +++++++++++++++++++ pkg/translate/terraform_provider/template.go | 10 ++++-- .../terraform_provider_file.go | 3 -- 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/assets/terraform/internal/manager/entry.go b/assets/terraform/internal/manager/entry.go index ab56a44e..21dc12fd 100644 --- a/assets/terraform/internal/manager/entry.go +++ b/assets/terraform/internal/manager/entry.go @@ -383,7 +383,7 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L } existing, err = o.service.List(ctx, location, "get", "", "") - if err != nil { + if err != nil && !sdkerrors.IsObjectNotFound(err) { return nil, fmt.Errorf("Failed to list remote entries: %w", err) } diff --git a/assets/terraform/internal/manager/uuid.go b/assets/terraform/internal/manager/uuid.go index 4227f3aa..e22cbaac 100644 --- a/assets/terraform/internal/manager/uuid.go +++ b/assets/terraform/internal/manager/uuid.go @@ -353,7 +353,7 @@ func (o *UuidObjectManager[E, L, S]) CreateMany(ctx context.Context, location L, } existing, err = o.service.List(ctx, location, "get", "", "") - if err != nil { + if err != nil && !sdkerrors.IsObjectNotFound(err) { return nil, fmt.Errorf("Failed to list remote entries: %w", err) } @@ -602,7 +602,7 @@ func (o *UuidObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L, } existing, err = o.service.List(ctx, location, "get", "", "") - if err != nil { + if err != nil && !sdkerrors.IsObjectNotFound(err) { return nil, fmt.Errorf("Failed to list remote entries: %w", err) } diff --git a/assets/terraform/test/resource_addresses_test.go b/assets/terraform/test/resource_addresses_test.go index f30301e3..a53908e6 100644 --- a/assets/terraform/test/resource_addresses_test.go +++ b/assets/terraform/test/resource_addresses_test.go @@ -30,6 +30,20 @@ func TestAccAddresses(t *testing.T) { ProtoV6ProviderFactories: testAccProviders, CheckDestroy: testAccAddressesDestroy(prefix), Steps: []resource.TestStep{ + { + Config: testAccAddressesResourceTmpl, + ConfigVariables: map[string]config.Variable{ + "prefix": config.StringVariable(prefix), + "addresses": config.MapVariable(map[string]config.Variable{})}, + }, + { + Config: testAccAddressesResourceTmpl, + ConfigVariables: map[string]config.Variable{ + "prefix": config.StringVariable(prefix), + "addresses": config.MapVariable(map[string]config.Variable{})}, + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, { Config: testAccAddressesResourceTmpl, ConfigVariables: map[string]config.Variable{ @@ -141,6 +155,20 @@ func TestAccAddresses(t *testing.T) { ), }, }, + { + Config: testAccAddressesResourceTmpl, + ConfigVariables: map[string]config.Variable{ + "prefix": config.StringVariable(prefix), + "addresses": config.MapVariable(map[string]config.Variable{})}, + }, + { + Config: testAccAddressesResourceTmpl, + ConfigVariables: map[string]config.Variable{ + "prefix": config.StringVariable(prefix), + "addresses": config.MapVariable(map[string]config.Variable{})}, + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, }, }) } diff --git a/assets/terraform/test/resource_security_policy_test.go b/assets/terraform/test/resource_security_policy_test.go index ab007921..0e0a77b9 100644 --- a/assets/terraform/test/resource_security_policy_test.go +++ b/assets/terraform/test/resource_security_policy_test.go @@ -471,6 +471,22 @@ func TestAccSecurityPolicyOrdering(t *testing.T) { ProtoV6ProviderFactories: testAccProviders, CheckDestroy: securityPolicyCheckDestroy(prefix, sdkLocation), Steps: []resource.TestStep{ + { + Config: securityPolicyOrderingTmpl, + ConfigVariables: map[string]config.Variable{ + "rule_names": config.ListVariable([]config.Variable{}...), + "location": cfgLocation, + }, + }, + { + Config: securityPolicyOrderingTmpl, + ConfigVariables: map[string]config.Variable{ + "rule_names": config.ListVariable([]config.Variable{}...), + "location": cfgLocation, + }, + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, { Config: securityPolicyOrderingTmpl, ConfigVariables: map[string]config.Variable{ @@ -523,6 +539,22 @@ func TestAccSecurityPolicyOrdering(t *testing.T) { ExpectServerSecurityRulesOrder(prefix, sdkLocation, rulesReordered), }, }, + { + Config: securityPolicyOrderingTmpl, + ConfigVariables: map[string]config.Variable{ + "rule_names": config.ListVariable([]config.Variable{}...), + "location": cfgLocation, + }, + }, + { + Config: securityPolicyOrderingTmpl, + ConfigVariables: map[string]config.Variable{ + "rule_names": config.ListVariable([]config.Variable{}...), + "location": cfgLocation, + }, + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, }, }) } diff --git a/pkg/translate/terraform_provider/template.go b/pkg/translate/terraform_provider/template.go index 13c5b580..566041eb 100644 --- a/pkg/translate/terraform_provider/template.go +++ b/pkg/translate/terraform_provider/template.go @@ -694,7 +694,7 @@ if resp.Diagnostics.HasError() { elements := make(map[string]{{ $resourceTFStructName }}) resp.Diagnostics.Append(state.{{ .ListAttribute.CamelCase }}.ElementsAs(ctx, &elements, false)...) -if resp.Diagnostics.HasError() { +if len(elements) == 0 || resp.Diagnostics.HasError() { return } @@ -786,6 +786,10 @@ if resp.Diagnostics.HasError() { var elements []{{ $resourceTFStructName }} state.{{ .ListAttribute.CamelCase }}.ElementsAs(ctx, &elements, false) +if len(elements) == 0 { + return +} + entries := make([]*{{ $resourceSDKStructName }}, 0, len(elements)) for _, elt := range elements { var entry *{{ $resourceSDKStructName }} @@ -949,7 +953,7 @@ for name, elt := range elements { } existing, err := r.manager.ReadMany(ctx, location, stateEntries) -if err != nil && !sdkerrors.IsObjectNotFound(err) { +if err != nil && !errors.Is(err, sdkmanager.ErrObjectNotFound) { resp.Diagnostics.AddError("Error while reading entries from the server", err.Error()) return } @@ -1044,7 +1048,7 @@ position := state.Position.CopyToPango() {{- end }} existing, err := r.manager.ReadMany(ctx, location, stateEntries, {{ $exhaustive }}) -if err != nil && !sdkerrors.IsObjectNotFound(err) { +if err != nil && !errors.Is(err, sdkmanager.ErrObjectNotFound) { resp.Diagnostics.AddError("Error while reading entries from the server", err.Error()) return } diff --git a/pkg/translate/terraform_provider/terraform_provider_file.go b/pkg/translate/terraform_provider/terraform_provider_file.go index 6e42fd65..dcbbe8d0 100644 --- a/pkg/translate/terraform_provider/terraform_provider_file.go +++ b/pkg/translate/terraform_provider/terraform_provider_file.go @@ -199,12 +199,9 @@ func (g *GenerateTerraformProvider) GenerateTerraformResource(resourceTyp proper switch resourceTyp { case properties.ResourceUuid: terraformProvider.ImportManager.AddSdkImport("github.com/PaloAltoNetworks/pango/rule", "") - terraformProvider.ImportManager.AddSdkImport("github.com/PaloAltoNetworks/pango/errors", "sdkerrors") case properties.ResourceEntry: case properties.ResourceUuidPlural: - terraformProvider.ImportManager.AddSdkImport("github.com/PaloAltoNetworks/pango/errors", "sdkerrors") case properties.ResourceEntryPlural: - terraformProvider.ImportManager.AddSdkImport("github.com/PaloAltoNetworks/pango/errors", "sdkerrors") case properties.ResourceCustom, properties.ResourceConfig: } From e9a6f46a2975a5b6fbcc8e2d95c9f3958860eed4 Mon Sep 17 00:00:00 2001 From: Krzysztof Klimonda Date: Wed, 22 Jan 2025 18:52:51 +0100 Subject: [PATCH 2/2] Fix ordering of create operations during uuid UpdateMany --- assets/terraform/internal/manager/uuid.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/assets/terraform/internal/manager/uuid.go b/assets/terraform/internal/manager/uuid.go index e22cbaac..630cd5dc 100644 --- a/assets/terraform/internal/manager/uuid.go +++ b/assets/terraform/internal/manager/uuid.go @@ -535,6 +535,8 @@ func (o *UuidObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L, } } + createOps := make([]*xmlapi.Config, len(planEntries)) + for _, elt := range processedStateEntries { path, err := location.XpathWithEntryName(o.client.Versioning(), elt.Entry.EntryName()) if err != nil { @@ -547,7 +549,14 @@ func (o *UuidObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L, } switch elt.State { - case entryMissing, entryOutdated: + case entryMissing: + createOps[elt.StateIdx] = &xmlapi.Config{ + Action: "edit", + Xpath: util.AsXpath(path), + Element: xmlEntry, + Target: o.client.GetTarget(), + } + case entryOutdated: updates.Add(&xmlapi.Config{ Action: "edit", Xpath: util.AsXpath(path), @@ -582,6 +591,12 @@ func (o *UuidObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L, } } + for _, elt := range createOps { + if elt != nil { + updates.Add(elt) + } + } + if len(updates.Operations) > 0 { if _, _, _, err := o.client.MultiConfig(ctx, updates, false, nil); err != nil { return nil, &Error{err: err, message: "failed to execute MultiConfig command"}