From 89d2e4680aaf6bdd56854f4e335c575ba5f30729 Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:48:19 -0700 Subject: [PATCH] Make Topic Name an immutable field * Make topic name immutable * Add tests --- apis/v1alpha1/ack-generate-metadata.yaml | 8 +++---- apis/v1alpha1/generator.yaml | 2 ++ generator.yaml | 2 ++ pkg/resource/platform_application/sdk.go | 4 +++- pkg/resource/platform_endpoint/sdk.go | 4 +++- pkg/resource/topic/hooks.go | 5 ++++ pkg/resource/topic/sdk.go | 24 +++++++++++++------ .../sdk_get_attributes_pre_set_output.go.tpl | 12 ++++------ test/e2e/tests/test_topic.py | 13 ++++++++++ 9 files changed, 54 insertions(+), 20 deletions(-) diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index 73da5ff..307dea1 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,13 +1,13 @@ ack_generate_info: - build_date: "2024-10-24T02:09:19Z" - build_hash: 36c2d234498c2bc4f60773ab8df632af4067f43b + build_date: "2024-10-25T16:48:58Z" + build_hash: ab15f9206796e9660c51695fab0ff07a09ea28e2 go_version: go1.23.2 - version: v0.39.1 + version: v0.39.1-2-gab15f92 api_directory_checksum: 07a2ec03f194442b69cf147a88a4cc33badaa895 api_version: v1alpha1 aws_sdk_go_version: v1.49.0 generator_config_info: - file_checksum: e95b5fd9a0571826102c9546eae4cc73228a8395 + file_checksum: da7d9b038cf0421583a14109fe0e04628911ab69 original_file_name: generator.yaml last_modification: reason: API generation diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index 049993b..77fa0eb 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -62,6 +62,8 @@ resources: sdk_get_attributes_post_set_output: template_path: hooks/topic/sdk_get_attributes_post_set_output.go.tpl fields: + Name: + is_immutable: true TopicArn: is_attribute: true is_arn: true diff --git a/generator.yaml b/generator.yaml index 049993b..77fa0eb 100644 --- a/generator.yaml +++ b/generator.yaml @@ -62,6 +62,8 @@ resources: sdk_get_attributes_post_set_output: template_path: hooks/topic/sdk_get_attributes_post_set_output.go.tpl fields: + Name: + is_immutable: true TopicArn: is_attribute: true is_arn: true diff --git a/pkg/resource/platform_application/sdk.go b/pkg/resource/platform_application/sdk.go index aff7666..4ddf4e5 100644 --- a/pkg/resource/platform_application/sdk.go +++ b/pkg/resource/platform_application/sdk.go @@ -308,7 +308,9 @@ func (rm *resourceManager) newSetAttributesRequestPayload( if r.ko.Spec.SuccessFeedbackSampleRate != nil { attrMap["SuccessFeedbackSampleRate"] = r.ko.Spec.SuccessFeedbackSampleRate } - res.SetAttributes(attrMap) + if len(attrMap) > 0 { + res.SetAttributes(attrMap) + } if r.ko.Status.ACKResourceMetadata != nil && r.ko.Status.ACKResourceMetadata.ARN != nil { res.SetPlatformApplicationArn(string(*r.ko.Status.ACKResourceMetadata.ARN)) } else { diff --git a/pkg/resource/platform_endpoint/sdk.go b/pkg/resource/platform_endpoint/sdk.go index 109bcbc..50d1063 100644 --- a/pkg/resource/platform_endpoint/sdk.go +++ b/pkg/resource/platform_endpoint/sdk.go @@ -265,7 +265,9 @@ func (rm *resourceManager) newSetAttributesRequestPayload( if r.ko.Spec.Token != nil { attrMap["Token"] = r.ko.Spec.Token } - res.SetAttributes(attrMap) + if len(attrMap) > 0 { + res.SetAttributes(attrMap) + } if r.ko.Status.EndpointARN != nil { res.SetEndpointArn(*r.ko.Status.EndpointARN) } diff --git a/pkg/resource/topic/hooks.go b/pkg/resource/topic/hooks.go index b3a2460..814965e 100644 --- a/pkg/resource/topic/hooks.go +++ b/pkg/resource/topic/hooks.go @@ -16,6 +16,7 @@ package topic import ( "context" "fmt" + "strings" ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1" ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" @@ -60,6 +61,10 @@ func (rm *resourceManager) customUpdate( defer func() { exit(err) }() + if immutableFieldChanges := rm.getImmutableFieldChanges(delta); len(immutableFieldChanges) > 0 { + msg := fmt.Sprintf("Immutable Spec fields have been modified: %s", strings.Join(immutableFieldChanges, ",")) + return nil, ackerr.NewTerminalError(fmt.Errorf(msg)) + } if delta.DifferentAt("Spec.Tags") { if err := rm.syncTags(ctx, desired, latest); err != nil { return nil, err diff --git a/pkg/resource/topic/sdk.go b/pkg/resource/topic/sdk.go index 37cdda2..177504a 100644 --- a/pkg/resource/topic/sdk.go +++ b/pkg/resource/topic/sdk.go @@ -103,17 +103,15 @@ func (rm *resourceManager) sdkFind( ko.Status.ACKResourceMetadata.ARN = &tmpARN ko.Spec.TracingConfig = resp.Attributes["TracingConfig"] - // If the Name field is empty, populate it with the last part of the topic ARN + // Populate the name field with the last part of the topic ARN // This is a workaround for the fact that the Name field is required by the // CreateTopic API call, but not by the GetTopicAttributes API call // Use case: adopting an existing topic by topic ARN - if ko.Spec.Name == nil { - topicName, err := rm.getTopicNameFromARN(tmpARN) - if err != nil { - return nil, err - } - ko.Spec.Name = &topicName + topicName, err := rm.getTopicNameFromARN(tmpARN) + if err != nil { + return nil, err } + ko.Spec.Name = &topicName rm.setStatusDefaults(ko) if tags, err := rm.getTags(ctx, r); err != nil { @@ -410,3 +408,15 @@ func (rm *resourceManager) terminalAWSError(err error) bool { return false } } + +// getImmutableFieldChanges returns list of immutable fields from the +func (rm *resourceManager) getImmutableFieldChanges( + delta *ackcompare.Delta, +) []string { + var fields []string + if delta.DifferentAt("Spec.Name") { + fields = append(fields, "Name") + } + + return fields +} diff --git a/templates/hooks/topic/sdk_get_attributes_pre_set_output.go.tpl b/templates/hooks/topic/sdk_get_attributes_pre_set_output.go.tpl index 4728589..556b933 100644 --- a/templates/hooks/topic/sdk_get_attributes_pre_set_output.go.tpl +++ b/templates/hooks/topic/sdk_get_attributes_pre_set_output.go.tpl @@ -1,13 +1,11 @@ - // If the Name field is empty, populate it with the last part of the topic ARN + // Populate the name field with the last part of the topic ARN // This is a workaround for the fact that the Name field is required by the // CreateTopic API call, but not by the GetTopicAttributes API call // Use case: adopting an existing topic by topic ARN - if ko.Spec.Name == nil { - topicName, err := rm.getTopicNameFromARN(tmpARN) - if err != nil { - return nil, err - } - ko.Spec.Name = &topicName + topicName, err := rm.getTopicNameFromARN(tmpARN) + if err != nil { + return nil, err } + ko.Spec.Name = &topicName \ No newline at end of file diff --git a/test/e2e/tests/test_topic.py b/test/e2e/tests/test_topic.py index 0ac749b..a3ae704 100644 --- a/test/e2e/tests/test_topic.py +++ b/test/e2e/tests/test_topic.py @@ -177,6 +177,19 @@ def test_crud(self, simple_topic): expect_after_update_tags, latest_tags, ) + updates = { + "spec": {"name": "my-simple-topic-edited"} + } + k8s.patch_custom_resource(ref, updates) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + k8s.wait_resource_consumed_by_controller(ref) + condition.assert_type_status(ref, condition.CONDITION_TYPE_TERMINAL) + + expected_msg = "Immutable Spec fields have been modified: Name" + terminal_condition = k8s.get_resource_condition(ref, condition.CONDITION_TYPE_TERMINAL) + # The name is immutable, testing if we get a terminal error + assert expected_msg in terminal_condition['message'] + def test_crud_fifo(self, fifo_topic): ref, res = fifo_topic