From 8d1fd26303ea6b945367297e13593ad0ff3e26ed Mon Sep 17 00:00:00 2001 From: "Kirill Sushkov (teeverr)" Date: Mon, 2 Oct 2023 14:24:35 +0200 Subject: [PATCH 1/4] remove spec.forProvider.name --- apis/servicecatalog/generator-config.yaml | 10 +- .../v1alpha1/zz_generated.deepcopy.go | 10 +- .../v1alpha1/zz_provisioned_product.go | 7 +- ...aws.crossplane.io_provisionedproducts.yaml | 9 +- .../provisionedproduct/setup.go | 93 ++++---- .../provisionedproduct/setup_test.go | 204 +++++++++++++++--- .../provisionedproduct/zz_controller.go | 4 +- .../provisionedproduct/zz_conversions.go | 150 +++++++------ 8 files changed, 307 insertions(+), 180 deletions(-) diff --git a/apis/servicecatalog/generator-config.yaml b/apis/servicecatalog/generator-config.yaml index c889753ca3..6217caaa4d 100644 --- a/apis/servicecatalog/generator-config.yaml +++ b/apis/servicecatalog/generator-config.yaml @@ -14,8 +14,11 @@ ignore: - ProvisionProductInput.ProvisionToken - UpdateProvisioningParameter.UsePreviousValue - UpdateProvisioningPreferences.StackSetOperationType + - ProvisionProductInput.ProvisionedProductName - UpdateProvisionedProductInput.ProvisionedProductId + - UpdateProvisionedProductInput.ProvisionedProductName - TerminateProvisionedProductInput.ProvisionedProductId + - TerminateProvisionedProductInput.ProvisionedProductName shape_names: - ProvisionedProductPlanSummary - ProvisionedProductPlanDetails @@ -95,9 +98,4 @@ resources: is_read_only: true from: operation: DescribeProvisionedProduct - path: ProvisionedProductDetail.StatusMessage - renames: - operations: - ProvisionProduct: - input_fields: - ProvisionedProductName: Name \ No newline at end of file + path: ProvisionedProductDetail.StatusMessage \ No newline at end of file diff --git a/apis/servicecatalog/v1alpha1/zz_generated.deepcopy.go b/apis/servicecatalog/v1alpha1/zz_generated.deepcopy.go index 546ae1f472..8d62aedb11 100644 --- a/apis/servicecatalog/v1alpha1/zz_generated.deepcopy.go +++ b/apis/servicecatalog/v1alpha1/zz_generated.deepcopy.go @@ -601,6 +601,11 @@ func (in *ProvisionedProductObservation) DeepCopyInto(out *ProvisionedProductObs *out = new(string) **out = **in } + if in.ProvisionedProductName != nil { + in, out := &in.ProvisionedProductName, &out.ProvisionedProductName + *out = new(string) + **out = **in + } if in.ProvisionedProductType != nil { in, out := &in.ProvisionedProductType, &out.ProvisionedProductType *out = new(string) @@ -672,11 +677,6 @@ func (in *ProvisionedProductParameters) DeepCopyInto(out *ProvisionedProductPara *out = new(string) **out = **in } - if in.Name != nil { - in, out := &in.Name, &out.Name - *out = new(string) - **out = **in - } if in.NotificationARNs != nil { in, out := &in.NotificationARNs, &out.NotificationARNs *out = make([]*string, len(*in)) diff --git a/apis/servicecatalog/v1alpha1/zz_provisioned_product.go b/apis/servicecatalog/v1alpha1/zz_provisioned_product.go index 0fed0ad1e7..ed25a03d19 100644 --- a/apis/servicecatalog/v1alpha1/zz_provisioned_product.go +++ b/apis/servicecatalog/v1alpha1/zz_provisioned_product.go @@ -37,11 +37,6 @@ type ProvisionedProductParameters struct { // // * zh - Chinese AcceptLanguage *string `json:"acceptLanguage,omitempty"` - // A user-friendly name for the provisioned product. This value must be unique - // for the Amazon Web Services account and cannot be updated after the product - // is provisioned. - // +kubebuilder:validation:Required - Name *string `json:"name"` // Passed to CloudFormation. The SNS topic ARNs to which to publish stack-related // events. NotificationARNs []*string `json:"notificationARNs,omitempty"` @@ -110,6 +105,8 @@ type ProvisionedProductObservation struct { Outputs map[string]*RecordOutput `json:"outputs,omitempty"` // The identifier of the provisioned product. ProvisionedProductID *string `json:"provisionedProductID,omitempty"` + // The user-friendly name of the provisioned product. + ProvisionedProductName *string `json:"provisionedProductName,omitempty"` // The type of provisioned product. The supported values are CFN_STACK and CFN_STACKSET. ProvisionedProductType *string `json:"provisionedProductType,omitempty"` // The errors that occurred. diff --git a/package/crds/servicecatalog.aws.crossplane.io_provisionedproducts.yaml b/package/crds/servicecatalog.aws.crossplane.io_provisionedproducts.yaml index 24b6d5ec71..9d7f214d54 100644 --- a/package/crds/servicecatalog.aws.crossplane.io_provisionedproducts.yaml +++ b/package/crds/servicecatalog.aws.crossplane.io_provisionedproducts.yaml @@ -73,11 +73,6 @@ spec: description: "The language code. \n * en - English (default) \n * jp - Japanese \n * zh - Chinese" type: string - name: - description: A user-friendly name for the provisioned product. - This value must be unique for the Amazon Web Services account - and cannot be updated after the product is provisioned. - type: string notificationARNs: description: Passed to CloudFormation. The SNS topic ARNs to which to publish stack-related events. @@ -162,7 +157,6 @@ spec: type: object type: array required: - - name - region type: object managementPolicies: @@ -419,6 +413,9 @@ spec: provisionedProductID: description: The identifier of the provisioned product. type: string + provisionedProductName: + description: The user-friendly name of the provisioned product. + type: string provisionedProductType: description: The type of provisioned product. The supported values are CFN_STACK and CFN_STACKSET. diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go index 3444d65864..4ba2fcffa4 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup.go @@ -63,6 +63,7 @@ const ( errCouldNotGetCFParameters = "could not get cloudformation stack parameters" errCouldNotDescribeRecord = "could not describe record" errCouldNotLookupProduct = "could not lookup product" + errCreatExternalNameIsNotValid = "external name is not equal provisioned product name" errAwsAPICodeInvalidParametersException = "Last Successful Provisioning Record doesn't exist." ) @@ -106,12 +107,12 @@ func SetupProvisionedProduct(mgr ctrl.Manager, o controller.Options) error { func prepareSetupExternal(cfClient *cfsdkv2.Client, kube client.Client) func(*external) { return func(e *external) { c := &custom{client: &clientset.CustomServiceCatalogClient{CfClient: cfClient, Client: e.client}, kube: kube} + e.preCreate = preCreate e.isUpToDate = c.isUpToDate e.lateInitialize = c.lateInitialize + e.preObserve = c.preObserve e.postObserve = c.postObserve e.preUpdate = c.preUpdate - e.preCreate = preCreate - e.postCreate = c.postCreate e.preDelete = preDelete } } @@ -132,7 +133,16 @@ func (c *custom) lateInitialize(spec *svcapitypes.ProvisionedProductParameters, return nil } -func (c *custom) isUpToDate(_ context.Context, ds *svcapitypes.ProvisionedProduct, resp *svcsdk.DescribeProvisionedProductOutput) (bool, string, error) { +func preCreate(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.ProvisionProductInput) error { + input.ProvisionToken = aws.String(genIdempotencyToken()) + if cr.GetName() != meta.GetExternalName(cr) { + return errors.New(errCreatExternalNameIsNotValid) + } + input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + return nil +} + +func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProduct, resp *svcsdk.DescribeProvisionedProductOutput) (bool, string, error) { // If the product is undergoing change, we want to assume that it is not up-to-date. This will force this resource // to be queued for an update (which will be skipped due to UNDER_CHANGE), and once that update fails, we will // recheck the status again. This will allow us to quickly transition from UNDER_CHANGE to AVAILABLE without having @@ -161,26 +171,34 @@ func (c *custom) isUpToDate(_ context.Context, ds *svcapitypes.ProvisionedProduc return false, "", errors.Wrap(err, errCouldNotGetCFParameters) } - productOrArtifactAreChanged, err := c.productOrArtifactAreChanged(&ds.Spec.ForProvider, resp.ProvisionedProductDetail) + productOrArtifactIsChanged, err := c.productOrArtifactIsChanged(&ds.Spec.ForProvider, resp.ProvisionedProductDetail) if err != nil { return false, "", errors.Wrap(err, "could not discover if product or artifact ids have changed") } - provisioningParamsAreChanged, err := c.provisioningParamsAreChanged(cfStackParameters, ds) + provisioningParamsAreChanged, err := c.provisioningParamsAreChanged(ctx, cfStackParameters, ds) if err != nil { return false, "", errors.Wrap(err, "could not compare provisioning parameters with previous ones") } - if productOrArtifactAreChanged || provisioningParamsAreChanged { + if productOrArtifactIsChanged || provisioningParamsAreChanged { return false, "", nil } return true, "", nil } +func (c *custom) preObserve(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.DescribeProvisionedProductInput) error { + if cr.GetName() == meta.GetExternalName(cr) { + input.Name = aws.String(meta.GetExternalName(cr)) + } else { + input.Id = aws.String(meta.GetExternalName(cr)) + } + return nil + +} func (c *custom) postObserve(_ context.Context, cr *svcapitypes.ProvisionedProduct, resp *svcsdk.DescribeProvisionedProductOutput, obs managed.ExternalObservation, err error) (managed.ExternalObservation, error) { if err != nil { return managed.ExternalObservation{}, err } - meta.SetExternalName(cr, *cr.Spec.ForProvider.Name) describeRecordInput := svcsdk.DescribeRecordInput{Id: resp.ProvisionedProductDetail.LastRecordId} describeRecordOutput, err := c.client.DescribeRecord(&describeRecordInput) @@ -214,47 +232,29 @@ func (c *custom) postObserve(_ context.Context, cr *svcapitypes.ProvisionedProdu return obs, nil } -func preCreate(_ context.Context, cr *svcapitypes.ProvisionedProduct, obj *svcsdk.ProvisionProductInput) error { - obj.ProvisionToken = aws.String(genIdempotencyToken()) - - // We want to specifically set this to match the forProvider name, as that - // is what we use to track the provisioned product - if cr.Spec.ForProvider.Name != nil { - meta.SetExternalName(cr, *cr.Spec.ForProvider.Name) - } - - return nil -} - -func (c *custom) postCreate(_ context.Context, cr *svcapitypes.ProvisionedProduct, obj *svcsdk.ProvisionProductOutput, cre managed.ExternalCreation, err error) (managed.ExternalCreation, error) { - if err != nil { - return cre, err - } - - // We are expected to set the external-name annotation upon creation since - // it can differ from the metadata.name for the ProvisionedProduct - if obj.RecordDetail != nil && obj.RecordDetail.ProvisionedProductName != nil { - meta.SetExternalName(cr, *obj.RecordDetail.ProvisionedProductName) - } - - return cre, nil -} - func (c *custom) preUpdate(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.UpdateProvisionedProductInput) error { if pointer.StringDeref(cr.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { return errors.New(errUpdatePending) } input.UpdateToken = aws.String(genIdempotencyToken()) - input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + if cr.GetName() == meta.GetExternalName(cr) { + input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + } else { + input.ProvisionedProductId = aws.String(meta.GetExternalName(cr)) + } return nil } -func preDelete(_ context.Context, cr *svcapitypes.ProvisionedProduct, obj *svcsdk.TerminateProvisionedProductInput) (bool, error) { +func preDelete(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.TerminateProvisionedProductInput) (bool, error) { if pointer.StringDeref(cr.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { return true, nil } - obj.TerminateToken = aws.String(genIdempotencyToken()) - obj.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + input.TerminateToken = aws.String(genIdempotencyToken()) + if cr.GetName() == meta.GetExternalName(cr) { + input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + } else { + input.ProvisionedProductId = aws.String(meta.GetExternalName(cr)) + } return false, nil } @@ -265,8 +265,8 @@ func setConditions(describeRecordOutput *svcsdk.DescribeRecordOutput, resp *svcs switch { case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_AVAILABLE): cr.SetConditions(xpv1.Available()) - case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "PROVISION_PRODUCT": - cr.SetConditions(xpv1.Creating()) + // case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "PROVISION_PRODUCT": + // cr.SetConditions(xpv1.Creating()) case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "UPDATE_PROVISIONED_PRODUCT": cr.SetConditions(xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange)) case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "TERMINATE_PROVISIONED_PRODUCT": @@ -280,16 +280,17 @@ func setConditions(describeRecordOutput *svcsdk.DescribeRecordOutput, resp *svcs } } -func (c *custom) provisioningParamsAreChanged(cfStackParams []cfsdkv2types.Parameter, ds *svcapitypes.ProvisionedProduct) (bool, error) { +func (c *custom) provisioningParamsAreChanged(ctx context.Context, cfStackParams []cfsdkv2types.Parameter, ds *svcapitypes.ProvisionedProduct) (bool, error) { nn := types.NamespacedName{ Name: ds.GetName(), } - mr := svcapitypes.ProvisionedProduct{} - err := c.kube.Get(context.TODO(), nn, &mr) + xr := svcapitypes.ProvisionedProduct{} + err := c.kube.Get(ctx, nn, &xr) if err != nil { return false, err } - if len(mr.Status.AtProvider.LastProvisioningParameters) != len(ds.Spec.ForProvider.ProvisioningParameters) { + // Product should be updated if amount of provisioning params from desired stats lesser than the amount from previous reconciliation loop + if len(xr.Status.AtProvider.LastProvisioningParameters) > len(ds.Spec.ForProvider.ProvisioningParameters) { return true, nil } @@ -303,6 +304,8 @@ func (c *custom) provisioningParamsAreChanged(cfStackParams []cfsdkv2types.Param // the desired state. Because on cloudformation side spaces are also trimmed if cfv, ok := cfStackKeyValue[*v.Key]; ok && strings.TrimSpace(pointer.StringDeref(v.Value, "")) == cfv { continue + } else if !ok { + return false, errors.Errorf("provisioning parameter %s is not present in cloud formation stack", *v.Key) } else { return true, nil } @@ -311,7 +314,7 @@ func (c *custom) provisioningParamsAreChanged(cfStackParams []cfsdkv2types.Param return false, nil } -func (c *custom) productOrArtifactAreChanged(ds *svcapitypes.ProvisionedProductParameters, resp *svcsdk.ProvisionedProductDetail) (bool, error) { +func (c *custom) productOrArtifactIsChanged(ds *svcapitypes.ProvisionedProductParameters, resp *svcsdk.ProvisionedProductDetail) (bool, error) { // ProvisioningArtifactID and ProvisioningArtifactName are mutual exclusive params, the same about ProductID and ProductName // But if describe a provisioned product aws api will return only IDs, so it's impossible to compare names with ids // Conditional statement below works only if desired state includes ProvisioningArtifactID and ProductID @@ -343,7 +346,7 @@ func (c *custom) getArtifactID(ds *svcapitypes.ProvisionedProductParameters) (st Id: ds.ProductID, Name: ds.ProductName, } - // DescribeProvisioningArtifact methods fits much better, but it has a bug + // DescribeProvisioningArtifact method fits much better, but it has a bug output, err := c.client.DescribeProduct(&input) if err != nil { return "", errors.Wrap(err, errCouldNotLookupProduct) diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go index a7c4e57db2..2a019ff87d 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go @@ -87,14 +87,14 @@ func describeProvisionedProduct(m ...describeProvisionedProductOutputModifier) * return output } -func prepareFakeExternal(fakeClient clientset.Client) func(*external) { +func prepareFakeExternal(fakeClient clientset.Client, kube client.Client) func(*external) { return func(e *external) { - c := &custom{client: fakeClient} + c := &custom{client: fakeClient, kube: kube} e.isUpToDate = c.isUpToDate e.lateInitialize = c.lateInitialize e.postObserve = c.postObserve - e.preCreate = c.preCreate - e.preDelete = c.preDelete + e.preCreate = preCreate + e.preDelete = preDelete e.preUpdate = c.preUpdate } } @@ -153,6 +153,13 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{} + return nil + }, + }, }, want: want{ result: false, @@ -198,6 +205,13 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{} + return nil + }, + }, }, want: want{ result: false, @@ -243,6 +257,13 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{} + return nil + }, + }, }, want: want{ result: false, @@ -287,6 +308,13 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{} + return nil + }, + }, }, want: want{ result: false, @@ -299,7 +327,7 @@ func TestIsUpToDate(t *testing.T) { withSpec(v1alpha1.ProvisionedProductParameters{ ProvisioningArtifactID: aws.String(provisioningArtifactID), ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ - {Key: aws.String("ParameterIsNotChanged"), Value: aws.String("true")}}, + {Key: aws.String("Parameter1"), Value: aws.String("quux")}}, }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ @@ -310,7 +338,12 @@ func TestIsUpToDate(t *testing.T) { }...), customClient: &fake.MockCustomServiceCatalogClient{ MockGetCloudformationStackParameters: func(provisionedProductOutputs []*svcsdk.RecordOutput) ([]cfsdkv2types.Parameter, error) { - return []cfsdkv2types.Parameter{{ParameterKey: aws.String("ParameterIsNotChanged"), ParameterValue: aws.String("false")}}, nil + return []cfsdkv2types.Parameter{ + {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, + {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter3"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter4"), ParameterValue: aws.String("product_default_value")}, + }, nil }, MockGetProvisionedProductOutputs: func(getPPInput *svcsdk.GetProvisionedProductOutputsInput) (*svcsdk.GetProvisionedProductOutputsOutput, error) { return &svcsdk.GetProvisionedProductOutputsOutput{}, nil @@ -329,20 +362,29 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + } + return nil + }, + }, }, want: want{ result: false, err: nil, }, }, - "NewParameterHasBeenAdded": { + "ParameterHasBeenAddedWithNewValue": { args: args{ provisionedProduct: provisionedProduct([]provisionedProductModifier{ withSpec(v1alpha1.ProvisionedProductParameters{ ProvisioningArtifactID: aws.String(provisioningArtifactID), ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ - {Key: aws.String("OldParameter"), Value: aws.String("foo")}, - {Key: aws.String("NewParameter"), Value: aws.String("bar")}, + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + {Key: aws.String("Parameter2"), Value: aws.String("quux")}, }, }), }...), @@ -355,7 +397,11 @@ func TestIsUpToDate(t *testing.T) { customClient: &fake.MockCustomServiceCatalogClient{ MockGetCloudformationStackParameters: func(provisionedProductOutputs []*svcsdk.RecordOutput) ([]cfsdkv2types.Parameter, error) { return []cfsdkv2types.Parameter{ - {ParameterKey: aws.String("OldParameter"), ParameterValue: aws.String("foo")}}, nil + {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, + {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter3"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter4"), ParameterValue: aws.String("product_default_value")}, + }, nil }, MockGetProvisionedProductOutputs: func(getPPInput *svcsdk.GetProvisionedProductOutputsInput) (*svcsdk.GetProvisionedProductOutputsOutput, error) { return &svcsdk.GetProvisionedProductOutputsOutput{}, nil @@ -374,19 +420,29 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + } + return nil + }, + }, }, want: want{ result: false, err: nil, }, }, - "ExistingParameterHasBeenRemoved": { + "ParameterHasBeenAddedWithDefaultValue": { args: args{ provisionedProduct: provisionedProduct([]provisionedProductModifier{ withSpec(v1alpha1.ProvisionedProductParameters{ ProvisioningArtifactID: aws.String(provisioningArtifactID), ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + {Key: aws.String("Parameter2"), Value: aws.String("product_default_value")}, }, }), }...), @@ -399,10 +455,69 @@ func TestIsUpToDate(t *testing.T) { customClient: &fake.MockCustomServiceCatalogClient{ MockGetCloudformationStackParameters: func(provisionedProductOutputs []*svcsdk.RecordOutput) ([]cfsdkv2types.Parameter, error) { return []cfsdkv2types.Parameter{ - {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, - {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("bar")}, + {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, + {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter3"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter4"), ParameterValue: aws.String("product_default_value")}, + }, nil + }, + MockGetProvisionedProductOutputs: func(getPPInput *svcsdk.GetProvisionedProductOutputsInput) (*svcsdk.GetProvisionedProductOutputsOutput, error) { + return &svcsdk.GetProvisionedProductOutputsOutput{}, nil + }, + MockDescribeProduct: func(dpInput *svcsdk.DescribeProductInput) (*svcsdk.DescribeProductOutput, error) { + return &svcsdk.DescribeProductOutput{ + ProductViewSummary: &svcsdk.ProductViewSummary{ + ProductId: aws.String("prod-fake"), + Name: aws.String("fake product"), }, - nil + ProvisioningArtifacts: []*svcsdk.ProvisioningArtifact{ + { + Id: aws.String(provisioningArtifactID), + }, + }, + }, nil + }, + }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + {Key: aws.String("Parameter2"), Value: aws.String("product_default_value")}, + } + return nil + }, + }, + }, + want: want{ + result: true, + err: nil, + }, + }, + "ExistingParameterHasBeenRemoved": { + args: args{ + provisionedProduct: provisionedProduct([]provisionedProductModifier{ + withSpec(v1alpha1.ProvisionedProductParameters{ + ProvisioningArtifactID: aws.String(provisioningArtifactID), + ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + }, + }), + }...), + describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ + withDetails(svcsdk.ProvisionedProductDetail{ + Id: aws.String("pp-fake"), + ProvisioningArtifactId: aws.String(provisioningArtifactID), + }), + }...), + customClient: &fake.MockCustomServiceCatalogClient{ + MockGetCloudformationStackParameters: func(provisionedProductOutputs []*svcsdk.RecordOutput) ([]cfsdkv2types.Parameter, error) { + return []cfsdkv2types.Parameter{ + {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, + {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("no_ways_to_determine_is_it_default_value_or_not")}, + {ParameterKey: aws.String("Parameter3"), ParameterValue: aws.String("product_default_value")}, + {ParameterKey: aws.String("Parameter4"), ParameterValue: aws.String("product_default_value")}, + }, nil }, MockGetProvisionedProductOutputs: func(getPPInput *svcsdk.GetProvisionedProductOutputsInput) (*svcsdk.GetProvisionedProductOutputsOutput, error) { return &svcsdk.GetProvisionedProductOutputsOutput{}, nil @@ -421,6 +536,16 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + {Key: aws.String("Parameter2"), Value: aws.String("no_ways_to_determine_is_it_default_value_or_not")}, + } + return nil + }, + }, }, want: want{ result: false, @@ -435,6 +560,7 @@ func TestIsUpToDate(t *testing.T) { ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter1"), Value: aws.String("foo")}, {Key: aws.String("Parameter2"), Value: aws.String("bar")}, + {Key: aws.String("Parameter3"), Value: aws.String("baz")}, }, }), }...), @@ -447,10 +573,11 @@ func TestIsUpToDate(t *testing.T) { customClient: &fake.MockCustomServiceCatalogClient{ MockGetCloudformationStackParameters: func(provisionedProductOutputs []*svcsdk.RecordOutput) ([]cfsdkv2types.Parameter, error) { return []cfsdkv2types.Parameter{ - {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, - {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("bar")}, - }, - nil + {ParameterKey: aws.String("Parameter1"), ParameterValue: aws.String("foo")}, + {ParameterKey: aws.String("Parameter2"), ParameterValue: aws.String("bar")}, + {ParameterKey: aws.String("Parameter3"), ParameterValue: aws.String("baz")}, + {ParameterKey: aws.String("Parameter4"), ParameterValue: aws.String("product_default_value")}, + }, nil }, MockGetProvisionedProductOutputs: func(getPPInput *svcsdk.GetProvisionedProductOutputsInput) (*svcsdk.GetProvisionedProductOutputsOutput, error) { return &svcsdk.GetProvisionedProductOutputsOutput{}, nil @@ -469,6 +596,17 @@ func TestIsUpToDate(t *testing.T) { }, nil }, }, + kube: &test.MockClient{ + MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + pp := obj.(*v1alpha1.ProvisionedProduct) + pp.Status.AtProvider.LastProvisioningParameters = []*v1alpha1.ProvisioningParameter{ + {Key: aws.String("Parameter1"), Value: aws.String("foo")}, + {Key: aws.String("Parameter2"), Value: aws.String("bar")}, + {Key: aws.String("Parameter2"), Value: aws.String("baz")}, + } + return nil + }, + }, }, want: want{ result: true, @@ -478,14 +616,14 @@ func TestIsUpToDate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient)} + opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} e := newExternal(tc.args.kube, tc.args.client, opts) - result, _, err := e.isUpToDate(nil, tc.args.provisionedProduct, tc.args.describeProvisionedProductOutput) + result, _, err := e.isUpToDate(context.TODO(), tc.args.provisionedProduct, tc.args.describeProvisionedProductOutput) if diff := cmp.Diff(err, tc.want.err, test.EquateErrors()); diff != "" { - t.Errorf("r: +want, -got:\n%s", diff) + t.Errorf("r: -want, +got:\n%s", diff) } - if diff := cmp.Diff(result, tc.want.result); diff != "" { - t.Errorf("r: +want, -got:\n%s", diff) + if diff := cmp.Diff(tc.want.result, result); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) } }) } @@ -526,11 +664,11 @@ func TestLateInitialize(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient)} + opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} e := newExternal(tc.args.kube, tc.args.client, opts) _ = e.lateInitialize(&tc.args.provisionedProduct.Spec.ForProvider, tc.args.describeProvisionedProductOutput) - if diff := cmp.Diff(*tc.args.provisionedProduct.Spec.ForProvider.AcceptLanguage, tc.want.acceptLanguage); diff != "" { - t.Errorf("r: +want, -got:\n%s", diff) + if diff := cmp.Diff(tc.want.acceptLanguage, *tc.args.provisionedProduct.Spec.ForProvider.AcceptLanguage); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) } }) } @@ -595,7 +733,7 @@ func TestPostObserve(t *testing.T) { }, }, want: want{ - status: xpv1.Unavailable().WithMessage(msgProvisionedProductStatusSdkUnderChange), + status: xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange), }, }, "StatusReconcileErrorProductError": { @@ -649,13 +787,13 @@ func TestPostObserve(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient)} + opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} e := newExternal(tc.args.kube, tc.args.client, opts) _, _ = e.postObserve(context.TODO(), tc.args.provisionedProduct, tc.args.describeProvisionedProductOutput, managed.ExternalObservation{}, nil) conditions := tc.args.provisionedProduct.Status.Conditions latestCondition := conditions[len(conditions)-1] - if diff := cmp.Diff(latestCondition, tc.want.status); diff != "" { - t.Errorf("r: +want, -got:\n%s", diff) + if diff := cmp.Diff(tc.want.status, latestCondition); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) } test.EquateConditions() }) @@ -699,11 +837,11 @@ func TestPreDelete(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient)} + opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} e := newExternal(tc.args.kube, tc.args.client, opts) ignore, _ := e.preDelete(context.TODO(), tc.args.provisionedProduct, &svcsdk.TerminateProvisionedProductInput{}) - if diff := cmp.Diff(ignore, tc.want.ignoreDeletion); diff != "" { - t.Errorf("r: +want, -got\n%s", diff) + if diff := cmp.Diff(tc.want.ignoreDeletion, ignore); diff != "" { + t.Errorf("r: -want, +got\n%s", diff) } }) diff --git a/pkg/controller/servicecatalog/provisionedproduct/zz_controller.go b/pkg/controller/servicecatalog/provisionedproduct/zz_controller.go index 344e2a527d..4ff454bf4a 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/zz_controller.go +++ b/pkg/controller/servicecatalog/provisionedproduct/zz_controller.go @@ -142,9 +142,9 @@ func (e *external) Create(ctx context.Context, mg cpresource.Managed) (managed.E cr.Status.AtProvider.ProvisionedProductID = nil } if resp.RecordDetail.ProvisionedProductName != nil { - cr.Spec.ForProvider.Name = resp.RecordDetail.ProvisionedProductName + cr.Status.AtProvider.ProvisionedProductName = resp.RecordDetail.ProvisionedProductName } else { - cr.Spec.ForProvider.Name = nil + cr.Status.AtProvider.ProvisionedProductName = nil } if resp.RecordDetail.ProvisionedProductType != nil { cr.Status.AtProvider.ProvisionedProductType = resp.RecordDetail.ProvisionedProductType diff --git a/pkg/controller/servicecatalog/provisionedproduct/zz_conversions.go b/pkg/controller/servicecatalog/provisionedproduct/zz_conversions.go index a223f8e864..e18a9653d3 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/zz_conversions.go +++ b/pkg/controller/servicecatalog/provisionedproduct/zz_conversions.go @@ -36,9 +36,6 @@ func GenerateDescribeProvisionedProductInput(cr *svcapitypes.ProvisionedProduct) if cr.Spec.ForProvider.AcceptLanguage != nil { res.SetAcceptLanguage(*cr.Spec.ForProvider.AcceptLanguage) } - if cr.Spec.ForProvider.Name != nil { - res.SetName(*cr.Spec.ForProvider.Name) - } return res } @@ -78,9 +75,6 @@ func GenerateProvisionProductInput(cr *svcapitypes.ProvisionedProduct) *svcsdk.P if cr.Spec.ForProvider.ProductName != nil { res.SetProductName(*cr.Spec.ForProvider.ProductName) } - if cr.Spec.ForProvider.Name != nil { - res.SetProvisionedProductName(*cr.Spec.ForProvider.Name) - } if cr.Spec.ForProvider.ProvisioningArtifactID != nil { res.SetProvisioningArtifactId(*cr.Spec.ForProvider.ProvisioningArtifactID) } @@ -88,66 +82,66 @@ func GenerateProvisionProductInput(cr *svcapitypes.ProvisionedProduct) *svcsdk.P res.SetProvisioningArtifactName(*cr.Spec.ForProvider.ProvisioningArtifactName) } if cr.Spec.ForProvider.ProvisioningParameters != nil { - f9 := []*svcsdk.ProvisioningParameter{} - for _, f9iter := range cr.Spec.ForProvider.ProvisioningParameters { - f9elem := &svcsdk.ProvisioningParameter{} - if f9iter.Key != nil { - f9elem.SetKey(*f9iter.Key) + f8 := []*svcsdk.ProvisioningParameter{} + for _, f8iter := range cr.Spec.ForProvider.ProvisioningParameters { + f8elem := &svcsdk.ProvisioningParameter{} + if f8iter.Key != nil { + f8elem.SetKey(*f8iter.Key) } - if f9iter.Value != nil { - f9elem.SetValue(*f9iter.Value) + if f8iter.Value != nil { + f8elem.SetValue(*f8iter.Value) } - f9 = append(f9, f9elem) + f8 = append(f8, f8elem) } - res.SetProvisioningParameters(f9) + res.SetProvisioningParameters(f8) } if cr.Spec.ForProvider.ProvisioningPreferences != nil { - f10 := &svcsdk.ProvisioningPreferences{} + f9 := &svcsdk.ProvisioningPreferences{} if cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts != nil { - f10f0 := []*string{} - for _, f10f0iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts { - var f10f0elem string - f10f0elem = *f10f0iter - f10f0 = append(f10f0, &f10f0elem) + f9f0 := []*string{} + for _, f9f0iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts { + var f9f0elem string + f9f0elem = *f9f0iter + f9f0 = append(f9f0, &f9f0elem) } - f10.SetStackSetAccounts(f10f0) + f9.SetStackSetAccounts(f9f0) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount != nil { - f10.SetStackSetFailureToleranceCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount) + f9.SetStackSetFailureToleranceCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage != nil { - f10.SetStackSetFailureTolerancePercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage) + f9.SetStackSetFailureTolerancePercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount != nil { - f10.SetStackSetMaxConcurrencyCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount) + f9.SetStackSetMaxConcurrencyCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage != nil { - f10.SetStackSetMaxConcurrencyPercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage) + f9.SetStackSetMaxConcurrencyPercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions != nil { - f10f5 := []*string{} - for _, f10f5iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions { - var f10f5elem string - f10f5elem = *f10f5iter - f10f5 = append(f10f5, &f10f5elem) + f9f5 := []*string{} + for _, f9f5iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions { + var f9f5elem string + f9f5elem = *f9f5iter + f9f5 = append(f9f5, &f9f5elem) } - f10.SetStackSetRegions(f10f5) + f9.SetStackSetRegions(f9f5) } - res.SetProvisioningPreferences(f10) + res.SetProvisioningPreferences(f9) } if cr.Spec.ForProvider.Tags != nil { - f11 := []*svcsdk.Tag{} - for _, f11iter := range cr.Spec.ForProvider.Tags { - f11elem := &svcsdk.Tag{} - if f11iter.Key != nil { - f11elem.SetKey(*f11iter.Key) + f10 := []*svcsdk.Tag{} + for _, f10iter := range cr.Spec.ForProvider.Tags { + f10elem := &svcsdk.Tag{} + if f10iter.Key != nil { + f10elem.SetKey(*f10iter.Key) } - if f11iter.Value != nil { - f11elem.SetValue(*f11iter.Value) + if f10iter.Value != nil { + f10elem.SetValue(*f10iter.Value) } - f11 = append(f11, f11elem) + f10 = append(f10, f10elem) } - res.SetTags(f11) + res.SetTags(f10) } return res @@ -179,66 +173,66 @@ func GenerateUpdateProvisionedProductInput(cr *svcapitypes.ProvisionedProduct) * res.SetProvisioningArtifactName(*cr.Spec.ForProvider.ProvisioningArtifactName) } if cr.Spec.ForProvider.ProvisioningParameters != nil { - f8 := []*svcsdk.UpdateProvisioningParameter{} - for _, f8iter := range cr.Spec.ForProvider.ProvisioningParameters { - f8elem := &svcsdk.UpdateProvisioningParameter{} - if f8iter.Key != nil { - f8elem.SetKey(*f8iter.Key) + f7 := []*svcsdk.UpdateProvisioningParameter{} + for _, f7iter := range cr.Spec.ForProvider.ProvisioningParameters { + f7elem := &svcsdk.UpdateProvisioningParameter{} + if f7iter.Key != nil { + f7elem.SetKey(*f7iter.Key) } - if f8iter.Value != nil { - f8elem.SetValue(*f8iter.Value) + if f7iter.Value != nil { + f7elem.SetValue(*f7iter.Value) } - f8 = append(f8, f8elem) + f7 = append(f7, f7elem) } - res.SetProvisioningParameters(f8) + res.SetProvisioningParameters(f7) } if cr.Spec.ForProvider.ProvisioningPreferences != nil { - f9 := &svcsdk.UpdateProvisioningPreferences{} + f8 := &svcsdk.UpdateProvisioningPreferences{} if cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts != nil { - f9f0 := []*string{} - for _, f9f0iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts { - var f9f0elem string - f9f0elem = *f9f0iter - f9f0 = append(f9f0, &f9f0elem) + f8f0 := []*string{} + for _, f8f0iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetAccounts { + var f8f0elem string + f8f0elem = *f8f0iter + f8f0 = append(f8f0, &f8f0elem) } - f9.SetStackSetAccounts(f9f0) + f8.SetStackSetAccounts(f8f0) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount != nil { - f9.SetStackSetFailureToleranceCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount) + f8.SetStackSetFailureToleranceCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureToleranceCount) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage != nil { - f9.SetStackSetFailureTolerancePercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage) + f8.SetStackSetFailureTolerancePercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetFailureTolerancePercentage) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount != nil { - f9.SetStackSetMaxConcurrencyCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount) + f8.SetStackSetMaxConcurrencyCount(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyCount) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage != nil { - f9.SetStackSetMaxConcurrencyPercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage) + f8.SetStackSetMaxConcurrencyPercentage(*cr.Spec.ForProvider.ProvisioningPreferences.StackSetMaxConcurrencyPercentage) } if cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions != nil { - f9f5 := []*string{} - for _, f9f5iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions { - var f9f5elem string - f9f5elem = *f9f5iter - f9f5 = append(f9f5, &f9f5elem) + f8f5 := []*string{} + for _, f8f5iter := range cr.Spec.ForProvider.ProvisioningPreferences.StackSetRegions { + var f8f5elem string + f8f5elem = *f8f5iter + f8f5 = append(f8f5, &f8f5elem) } - f9.SetStackSetRegions(f9f5) + f8.SetStackSetRegions(f8f5) } - res.SetProvisioningPreferences(f9) + res.SetProvisioningPreferences(f8) } if cr.Spec.ForProvider.Tags != nil { - f10 := []*svcsdk.Tag{} - for _, f10iter := range cr.Spec.ForProvider.Tags { - f10elem := &svcsdk.Tag{} - if f10iter.Key != nil { - f10elem.SetKey(*f10iter.Key) + f9 := []*svcsdk.Tag{} + for _, f9iter := range cr.Spec.ForProvider.Tags { + f9elem := &svcsdk.Tag{} + if f9iter.Key != nil { + f9elem.SetKey(*f9iter.Key) } - if f10iter.Value != nil { - f10elem.SetValue(*f10iter.Value) + if f9iter.Value != nil { + f9elem.SetValue(*f9iter.Value) } - f10 = append(f10, f10elem) + f9 = append(f9, f9elem) } - res.SetTags(f10) + res.SetTags(f9) } return res From e0550ee1ecbd6066d4766ba3c08b9d1ebbca6274 Mon Sep 17 00:00:00 2001 From: "Kirill Sushkov (teeverr)" Date: Mon, 2 Oct 2023 17:02:17 +0200 Subject: [PATCH 2/4] refactore setConditions --- .../provisionedproduct/setup.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go index 4ba2fcffa4..80ec47e0e9 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup.go @@ -185,6 +185,7 @@ func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProd } return true, "", nil } + func (c *custom) preObserve(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.DescribeProvisionedProductInput) error { if cr.GetName() == meta.GetExternalName(cr) { input.Name = aws.String(meta.GetExternalName(cr)) @@ -259,18 +260,20 @@ func preDelete(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svc } func setConditions(describeRecordOutput *svcsdk.DescribeRecordOutput, resp *svcsdk.DescribeProvisionedProductOutput, cr *svcapitypes.ProvisionedProduct) { - recordType := pointer.StringDeref(describeRecordOutput.RecordDetail.RecordType, "UPDATE_PROVISIONED_PRODUCT") - ppStatus := aws.StringValue(resp.ProvisionedProductDetail.Status) switch { case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_AVAILABLE): cr.SetConditions(xpv1.Available()) - // case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "PROVISION_PRODUCT": - // cr.SetConditions(xpv1.Creating()) - case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "UPDATE_PROVISIONED_PRODUCT": - cr.SetConditions(xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange)) - case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) && recordType == "TERMINATE_PROVISIONED_PRODUCT": - cr.SetConditions(xpv1.Deleting()) + case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE): + recordType := pointer.StringDeref(describeRecordOutput.RecordDetail.RecordType, "UPDATE_PROVISIONED_PRODUCT") + switch { + case recordType == "PROVISION_PRODUCT": + cr.SetConditions(xpv1.Creating()) + case recordType == "UPDATE_PROVISIONED_PRODUCT": + cr.SetConditions(xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange)) + case recordType == "TERMINATE_PROVISIONED_PRODUCT": + cr.SetConditions(xpv1.Deleting()) + } case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_PLAN_IN_PROGRESS): cr.SetConditions(xpv1.Unavailable().WithMessage(msgProvisionedProductStatusSdkPlanInProgress)) case ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_ERROR): From 726b81d3eceb38db4f4bb5af6f9ad7ba6fd1c9d4 Mon Sep 17 00:00:00 2001 From: "Kirill Sushkov (teeverr)" Date: Tue, 3 Oct 2023 12:34:20 +0200 Subject: [PATCH 3/4] fix http resp code 400 in the start of creating --- .../provisionedproduct/setup.go | 43 ++++++++----------- .../provisionedproduct/setup_test.go | 40 ++++++++++++++++- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go index 80ec47e0e9..2198d0ec6b 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup.go @@ -19,17 +19,15 @@ package provisionedproduct import ( "context" "fmt" + "reflect" "strings" - "k8s.io/apimachinery/pkg/types" - awsconfig "github.com/aws/aws-sdk-go-v2/config" cfsdkv2 "github.com/aws/aws-sdk-go-v2/service/cloudformation" cfsdkv2types "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" svcsdk "github.com/aws/aws-sdk-go/service/servicecatalog" - awsclient "github.com/crossplane-contrib/provider-aws/pkg/clients" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/connection" "github.com/crossplane/crossplane-runtime/pkg/controller" @@ -40,12 +38,14 @@ import ( "github.com/google/uuid" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" svcapitypes "github.com/crossplane-contrib/provider-aws/apis/servicecatalog/v1alpha1" "github.com/crossplane-contrib/provider-aws/apis/v1alpha1" + awsclient "github.com/crossplane-contrib/provider-aws/pkg/clients" clientset "github.com/crossplane-contrib/provider-aws/pkg/clients/servicecatalog" "github.com/crossplane-contrib/provider-aws/pkg/features" ) @@ -58,7 +58,6 @@ const ( msgProvisionedProductStatusSdkPlanInProgress = "provisioned product is awaiting plan approval" msgProvisionedProductStatusSdkError = "provisioned product has status ERROR" - errUpdatePending = "provisioned product is already under change, not updating" errCouldNotGetProvisionedProductOutputs = "could not get provisioned product outputs" errCouldNotGetCFParameters = "could not get cloudformation stack parameters" errCouldNotDescribeRecord = "could not describe record" @@ -108,11 +107,11 @@ func prepareSetupExternal(cfClient *cfsdkv2.Client, kube client.Client) func(*ex return func(e *external) { c := &custom{client: &clientset.CustomServiceCatalogClient{CfClient: cfClient, Client: e.client}, kube: kube} e.preCreate = preCreate + e.preUpdate = c.preUpdate e.isUpToDate = c.isUpToDate e.lateInitialize = c.lateInitialize e.preObserve = c.preObserve e.postObserve = c.postObserve - e.preUpdate = c.preUpdate e.preDelete = preDelete } } @@ -142,12 +141,23 @@ func preCreate(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svc return nil } -func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProduct, resp *svcsdk.DescribeProvisionedProductOutput) (bool, string, error) { +func (c *custom) preUpdate(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.UpdateProvisionedProductInput) error { + input.UpdateToken = aws.String(genIdempotencyToken()) + if cr.GetName() == meta.GetExternalName(cr) { + input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) + } else { + input.ProvisionedProductId = aws.String(meta.GetExternalName(cr)) + } + return nil +} + +func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProduct, resp *svcsdk.DescribeProvisionedProductOutput) (bool, string, error) { // nolint:gocyclo // If the product is undergoing change, we want to assume that it is not up-to-date. This will force this resource // to be queued for an update (which will be skipped due to UNDER_CHANGE), and once that update fails, we will // recheck the status again. This will allow us to quickly transition from UNDER_CHANGE to AVAILABLE without having // to wait for the entire polling interval to pass before re-checking the status. - if pointer.StringDeref(ds.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { + if pointer.StringDeref(ds.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) || + reflect.ValueOf(ds.Status.AtProvider).IsZero() { return true, "", nil } @@ -158,10 +168,8 @@ func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProd // is wrong with the provisioned product (error on creation, tainted, etc) // We will be able to handle those specific cases in postObserve var aerr awserr.Error - if ok := errors.As(err, &aerr); ok { - if aerr.Code() == svcsdk.ErrCodeInvalidParametersException && aerr.Message() == errAwsAPICodeInvalidParametersException { - return false, "", nil - } + if ok := errors.As(err, &aerr); ok && aerr.Code() == svcsdk.ErrCodeInvalidParametersException && aerr.Message() == errAwsAPICodeInvalidParametersException { + return false, "", nil } return false, "", errors.Wrap(err, errCouldNotGetProvisionedProductOutputs) } @@ -233,19 +241,6 @@ func (c *custom) postObserve(_ context.Context, cr *svcapitypes.ProvisionedProdu return obs, nil } -func (c *custom) preUpdate(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.UpdateProvisionedProductInput) error { - if pointer.StringDeref(cr.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { - return errors.New(errUpdatePending) - } - input.UpdateToken = aws.String(genIdempotencyToken()) - if cr.GetName() == meta.GetExternalName(cr) { - input.ProvisionedProductName = aws.String(meta.GetExternalName(cr)) - } else { - input.ProvisionedProductId = aws.String(meta.GetExternalName(cr)) - } - return nil -} - func preDelete(_ context.Context, cr *svcapitypes.ProvisionedProduct, input *svcsdk.TerminateProvisionedProductInput) (bool, error) { if pointer.StringDeref(cr.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { return true, nil diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go index 2a019ff87d..68c9b49a36 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go @@ -123,6 +123,10 @@ func TestIsUpToDate(t *testing.T) { ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter"), Value: aws.String("foo")}}, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -175,6 +179,10 @@ func TestIsUpToDate(t *testing.T) { ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter"), Value: aws.String("foo")}}, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -227,6 +235,10 @@ func TestIsUpToDate(t *testing.T) { ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter"), Value: aws.String("foo")}}, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -279,6 +291,10 @@ func TestIsUpToDate(t *testing.T) { ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ {Key: aws.String("Parameter"), Value: aws.String("foo")}}, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -327,7 +343,11 @@ func TestIsUpToDate(t *testing.T) { withSpec(v1alpha1.ProvisionedProductParameters{ ProvisioningArtifactID: aws.String(provisioningArtifactID), ProvisioningParameters: []*v1alpha1.ProvisioningParameter{ - {Key: aws.String("Parameter1"), Value: aws.String("quux")}}, + {Key: aws.String("Parameter1"), Value: aws.String("bar")}}, + }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ @@ -387,6 +407,10 @@ func TestIsUpToDate(t *testing.T) { {Key: aws.String("Parameter2"), Value: aws.String("quux")}, }, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -445,6 +469,10 @@ func TestIsUpToDate(t *testing.T) { {Key: aws.String("Parameter2"), Value: aws.String("product_default_value")}, }, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -503,6 +531,10 @@ func TestIsUpToDate(t *testing.T) { {Key: aws.String("Parameter1"), Value: aws.String("foo")}, }, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -563,6 +595,10 @@ func TestIsUpToDate(t *testing.T) { {Key: aws.String("Parameter3"), Value: aws.String("baz")}, }, }), + withStatus(v1alpha1.ProvisionedProductStatus{ + AtProvider: v1alpha1.ProvisionedProductObservation{ + Status: aws.String(string(v1alpha1.ProvisionedProductStatus_SDK_AVAILABLE))}, + }), }...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ withDetails(svcsdk.ProvisionedProductDetail{ @@ -712,7 +748,7 @@ func TestPostObserve(t *testing.T) { status: xpv1.Available(), }, }, - "StatusUnavailableWithAmendment": { + "StatusAvailableWithAmendment": { args: args{ provisionedProduct: provisionedProduct([]provisionedProductModifier{provisionedProductStatus}...), describeProvisionedProductOutput: describeProvisionedProduct([]describeProvisionedProductOutputModifier{ From cbc7f957a2648a7b83f4a21a334804d94cd0ef8c Mon Sep 17 00:00:00 2001 From: "Kirill Sushkov (teeverr)" Date: Thu, 5 Oct 2023 13:24:13 +0200 Subject: [PATCH 4/4] fix http resp code 400 in the start of creating, isUpToDate fixed --- .../servicecatalog/provisionedproduct/setup.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go index 2198d0ec6b..ef3498d67d 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup.go @@ -19,7 +19,6 @@ package provisionedproduct import ( "context" "fmt" - "reflect" "strings" awsconfig "github.com/aws/aws-sdk-go-v2/config" @@ -108,8 +107,8 @@ func prepareSetupExternal(cfClient *cfsdkv2.Client, kube client.Client) func(*ex c := &custom{client: &clientset.CustomServiceCatalogClient{CfClient: cfClient, Client: e.client}, kube: kube} e.preCreate = preCreate e.preUpdate = c.preUpdate - e.isUpToDate = c.isUpToDate e.lateInitialize = c.lateInitialize + e.isUpToDate = c.isUpToDate e.preObserve = c.preObserve e.postObserve = c.postObserve e.preDelete = preDelete @@ -156,8 +155,7 @@ func (c *custom) isUpToDate(ctx context.Context, ds *svcapitypes.ProvisionedProd // to be queued for an update (which will be skipped due to UNDER_CHANGE), and once that update fails, we will // recheck the status again. This will allow us to quickly transition from UNDER_CHANGE to AVAILABLE without having // to wait for the entire polling interval to pass before re-checking the status. - if pointer.StringDeref(ds.Status.AtProvider.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) || - reflect.ValueOf(ds.Status.AtProvider).IsZero() { + if pointer.StringDeref(resp.ProvisionedProductDetail.Status, "") == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) { return true, "", nil } @@ -294,7 +292,9 @@ func (c *custom) provisioningParamsAreChanged(ctx context.Context, cfStackParams cfStackKeyValue := make(map[string]string) for _, v := range cfStackParams { - cfStackKeyValue[*v.ParameterKey] = pointer.StringDeref(v.ParameterValue, "") + if v.ParameterKey != nil { + cfStackKeyValue[*v.ParameterKey] = pointer.StringDeref(v.ParameterValue, "") + } } for _, v := range ds.Spec.ForProvider.ProvisioningParameters { @@ -353,7 +353,7 @@ func (c *custom) getArtifactID(ds *svcapitypes.ProvisionedProductParameters) (st for _, artifact := range output.ProvisioningArtifacts { if pointer.StringDeref(ds.ProvisioningArtifactName, "") == *artifact.Name || pointer.StringDeref(ds.ProvisioningArtifactID, "") == *artifact.Id { - return pointer.StringDeref(artifact.Id, ""), nil + return *artifact.Id, nil } } return "", errors.Wrap(errors.New("artifact not found"), errCouldNotLookupProduct)