From 84492a0345724faeb03f85ed4b6e59825439ade6 Mon Sep 17 00:00:00 2001 From: Nishant Burte Date: Fri, 23 Jun 2023 12:31:05 -0700 Subject: [PATCH] Route53: HostedZone spec should have tags support Issue (aws-controllers-k8s/community#1554) Description of changes: Adds support of tags in HostedZone spec By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --- apis/v1alpha1/ack-generate-metadata.yaml | 12 +- apis/v1alpha1/generator.yaml | 20 +++ apis/v1alpha1/hosted_zone.go | 6 + apis/v1alpha1/types.go | 12 ++ apis/v1alpha1/zz_generated.deepcopy.go | 62 +++++++ .../route53.services.k8s.aws_hostedzones.yaml | 16 ++ generator.yaml | 20 +++ .../route53.services.k8s.aws_hostedzones.yaml | 16 ++ helm/templates/deployment.yaml | 15 +- helm/values.schema.json | 9 + helm/values.yaml | 22 ++- pkg/resource/hosted_zone/delta.go | 1 + pkg/resource/hosted_zone/hook.go | 160 ++++++++++++++++++ pkg/resource/hosted_zone/manager.go | 12 ++ pkg/resource/hosted_zone/sdk.go | 32 +++- pkg/resource/hosted_zone/tags.go | 76 +++++++++ .../sdk_create_post_set_output.go.tpl | 12 ++ .../hooks/hosted_zone/sdk_ensure_tags.go.tpl | 13 ++ .../hooks/hosted_zone/sdk_file_end.go.tpl | 23 +++ .../sdk_read_one_post_set_output.go.tpl | 3 + test/e2e/conftest.py | 3 + test/e2e/requirements.txt | 2 +- test/e2e/resources/hosted_zone_public.yaml | 3 + test/e2e/tests/helper.py | 46 +++++ test/e2e/tests/test_hosted_zone.py | 135 +++++++++++++-- 25 files changed, 710 insertions(+), 21 deletions(-) create mode 100644 pkg/resource/hosted_zone/tags.go create mode 100644 templates/hooks/hosted_zone/sdk_create_post_set_output.go.tpl create mode 100644 templates/hooks/hosted_zone/sdk_ensure_tags.go.tpl create mode 100644 templates/hooks/hosted_zone/sdk_file_end.go.tpl create mode 100644 templates/hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl create mode 100644 test/e2e/tests/helper.py diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index 4f8ce9f..8c23adf 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,13 +1,13 @@ ack_generate_info: - build_date: "2023-05-15T23:45:27Z" - build_hash: 8f3ba427974fd6e769926778d54834eaee3b81a3 - go_version: go1.19 - version: v0.26.1 -api_directory_checksum: e2bcdcfd015dd5528e4df8745ede3ffccfd4e683 + build_date: "2023-07-17T18:30:35Z" + build_hash: e9b68590da73ce9143ba1e4361cebdc1d876c81e + go_version: go1.20.3 + version: v0.26.1-7-ge9b6859 +api_directory_checksum: 2f681761d3417cd3c65709b6dc71d9853bdf0481 api_version: v1alpha1 aws_sdk_go_version: v1.44.93 generator_config_info: - file_checksum: 554ef2519945f32ea8788a1add7a3c496c944d88 + file_checksum: e4f89494b4e355fb756095b55c95050a23efa0b3 original_file_name: generator.yaml last_modification: reason: API generation diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index c86ddb4..2844e51 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -30,8 +30,28 @@ resources: - InvalidDomainName - InvalidInput - InvalidVPCId + fields: + Tags: + from: + operation: ChangeTagsForResource + path: AddTags + compare: + is_ignored: True hooks: + delta_pre_compare: + code: compareTags(delta, a, b) + sdk_read_one_post_set_output: + template_path: hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl + sdk_create_post_set_output: + template_path: hooks/hosted_zone/sdk_create_post_set_output.go.tpl sdk_create_post_build_request: template_path: hooks/hosted_zone/sdk_create_post_build_request.go.tpl + sdk_file_end: + template_path: hooks/hosted_zone/sdk_file_end.go.tpl + ensure_tags: + template_path: hooks/hosted_zone/sdk_ensure_tags.go.tpl + tags: ignore: true + update_operation: + custom_method_name: customUpdateHostedZone diff --git a/apis/v1alpha1/hosted_zone.go b/apis/v1alpha1/hosted_zone.go index 86591f6..dfdd1ed 100644 --- a/apis/v1alpha1/hosted_zone.go +++ b/apis/v1alpha1/hosted_zone.go @@ -50,6 +50,12 @@ type HostedZoneSpec struct { // NameServers that CreateHostedZone returns in DelegationSet. // +kubebuilder:validation:Required Name *string `json:"name"` + // A complex type that contains a list of the tags that you want to add to the + // specified health check or hosted zone and/or the tags that you want to edit + // Value for. + // + // You can add a maximum of 10 tags to a health check or a hosted zone. + Tags []*Tag `json:"tags,omitempty"` // (Private hosted zones only) A complex type that contains information about // the Amazon VPC that you're associating with this hosted zone. // diff --git a/apis/v1alpha1/types.go b/apis/v1alpha1/types.go index d8fe3b6..b35bc1b 100644 --- a/apis/v1alpha1/types.go +++ b/apis/v1alpha1/types.go @@ -138,12 +138,24 @@ type ResourceRecordSet struct { Name *string `json:"name,omitempty"` } +// A complex type containing a resource and its associated tags. +type ResourceTagSet struct { + Tags []*Tag `json:"tags,omitempty"` +} + // A complex type that contains the status that one Amazon Route 53 health checker // reports and the time of the health check. type StatusReport struct { CheckedTime *metav1.Time `json:"checkedTime,omitempty"` } +// A complex type that contains information about a tag that you want to add +// or edit for the specified health check or hosted zone. +type Tag struct { + Key *string `json:"key,omitempty"` + Value *string `json:"value,omitempty"` +} + // A complex type that contains settings for the new traffic policy instance. type TrafficPolicyInstance struct { HostedZoneID *string `json:"hostedZoneID,omitempty"` diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index c0cd1b6..574040e 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -262,6 +262,17 @@ func (in *HostedZoneSpec) DeepCopyInto(out *HostedZoneSpec) { *out = new(string) **out = **in } + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make([]*Tag, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Tag) + (*in).DeepCopyInto(*out) + } + } + } if in.VPC != nil { in, out := &in.VPC, &out.VPC *out = new(VPC) @@ -493,6 +504,32 @@ func (in *ResourceRecordSet) DeepCopy() *ResourceRecordSet { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResourceTagSet) DeepCopyInto(out *ResourceTagSet) { + *out = *in + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make([]*Tag, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Tag) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResourceTagSet. +func (in *ResourceTagSet) DeepCopy() *ResourceTagSet { + if in == nil { + return nil + } + out := new(ResourceTagSet) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StatusReport) DeepCopyInto(out *StatusReport) { *out = *in @@ -512,6 +549,31 @@ func (in *StatusReport) DeepCopy() *StatusReport { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Tag) DeepCopyInto(out *Tag) { + *out = *in + if in.Key != nil { + in, out := &in.Key, &out.Key + *out = new(string) + **out = **in + } + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Tag. +func (in *Tag) DeepCopy() *Tag { + if in == nil { + return nil + } + out := new(Tag) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TrafficPolicyInstance) DeepCopyInto(out *TrafficPolicyInstance) { *out = *in diff --git a/config/crd/bases/route53.services.k8s.aws_hostedzones.yaml b/config/crd/bases/route53.services.k8s.aws_hostedzones.yaml index 06fed93..bb2af2c 100644 --- a/config/crd/bases/route53.services.k8s.aws_hostedzones.yaml +++ b/config/crd/bases/route53.services.k8s.aws_hostedzones.yaml @@ -67,6 +67,22 @@ spec: your domain to the set of NameServers that CreateHostedZone returns in DelegationSet." type: string + tags: + description: "A complex type that contains a list of the tags that + you want to add to the specified health check or hosted zone and/or + the tags that you want to edit Value for. \n You can add a maximum + of 10 tags to a health check or a hosted zone." + items: + description: A complex type that contains information about a tag + that you want to add or edit for the specified health check or + hosted zone. + properties: + key: + type: string + value: + type: string + type: object + type: array vpc: description: "(Private hosted zones only) A complex type that contains information about the Amazon VPC that you're associating with this diff --git a/generator.yaml b/generator.yaml index c86ddb4..2844e51 100644 --- a/generator.yaml +++ b/generator.yaml @@ -30,8 +30,28 @@ resources: - InvalidDomainName - InvalidInput - InvalidVPCId + fields: + Tags: + from: + operation: ChangeTagsForResource + path: AddTags + compare: + is_ignored: True hooks: + delta_pre_compare: + code: compareTags(delta, a, b) + sdk_read_one_post_set_output: + template_path: hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl + sdk_create_post_set_output: + template_path: hooks/hosted_zone/sdk_create_post_set_output.go.tpl sdk_create_post_build_request: template_path: hooks/hosted_zone/sdk_create_post_build_request.go.tpl + sdk_file_end: + template_path: hooks/hosted_zone/sdk_file_end.go.tpl + ensure_tags: + template_path: hooks/hosted_zone/sdk_ensure_tags.go.tpl + tags: ignore: true + update_operation: + custom_method_name: customUpdateHostedZone diff --git a/helm/crds/route53.services.k8s.aws_hostedzones.yaml b/helm/crds/route53.services.k8s.aws_hostedzones.yaml index 1b9a676..8e9b122 100644 --- a/helm/crds/route53.services.k8s.aws_hostedzones.yaml +++ b/helm/crds/route53.services.k8s.aws_hostedzones.yaml @@ -67,6 +67,22 @@ spec: your domain to the set of NameServers that CreateHostedZone returns in DelegationSet." type: string + tags: + description: "A complex type that contains a list of the tags that + you want to add to the specified health check or hosted zone and/or + the tags that you want to edit Value for. \n You can add a maximum + of 10 tags to a health check or a hosted zone." + items: + description: A complex type that contains information about a tag + that you want to add or edit for the specified health check or + hosted zone. + properties: + key: + type: string + value: + type: string + type: object + type: array vpc: description: "(Private hosted zones only) A complex type that contains information about the Amazon VPC that you're associating with this diff --git a/helm/templates/deployment.yaml b/helm/templates/deployment.yaml index 7504a61..4d087a5 100644 --- a/helm/templates/deployment.yaml +++ b/helm/templates/deployment.yaml @@ -18,10 +18,12 @@ spec: app.kubernetes.io/instance: {{ .Release.Name }} template: metadata: +{{- if .Values.deployment.annotations }} annotations: {{- range $key, $value := .Values.deployment.annotations }} {{ $key }}: {{ $value | quote }} {{- end }} +{{- end }} labels: app.kubernetes.io/name: {{ include "app.name" . }} app.kubernetes.io/instance: {{ .Release.Name }} @@ -104,11 +106,19 @@ spec: value: {{ include "aws.credentials.path" . }} - name: AWS_PROFILE value: {{ .Values.aws.credentials.profile }} + {{- end }} + {{- if .Values.deployment.extraEnvVars -}} + {{ toYaml .Values.deployment.extraEnvVars | nindent 8 }} + {{- end }} volumeMounts: + {{- if .Values.aws.credentials.secretName }} - name: {{ .Values.aws.credentials.secretName }} mountPath: {{ include "aws.credentials.secret_mount_path" . }} readOnly: true {{- end }} + {{- if .Values.deployment.extraVolumeMounts -}} + {{ toYaml .Values.deployment.extraVolumeMounts | nindent 12 }} + {{- end }} securityContext: allowPrivilegeEscalation: false privileged: false @@ -133,9 +143,12 @@ spec: hostIPC: false hostNetwork: false hostPID: false - {{ if .Values.aws.credentials.secretName -}} volumes: + {{- if .Values.aws.credentials.secretName -}} - name: {{ .Values.aws.credentials.secretName }} secret: secretName: {{ .Values.aws.credentials.secretName }} {{ end -}} +{{- if .Values.deployment.extraVolumes }} +{{ toYaml .Values.deployment.extraVolumes | indent 8}} +{{- end }} diff --git a/helm/values.schema.json b/helm/values.schema.json index 79fd18c..fb4437b 100644 --- a/helm/values.schema.json +++ b/helm/values.schema.json @@ -58,6 +58,15 @@ }, "priorityClassName": { "type": "string" + }, + "extraVolumeMounts": { + "type": "array" + }, + "extraVolumes": { + "type": "array" + }, + "extraEnvVars": { + "type": "array" } }, "required": [ diff --git a/helm/values.yaml b/helm/values.yaml index 1498e41..add1f01 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -28,6 +28,26 @@ deployment: # Which priorityClassName to set? # See: https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#pod-priority priorityClassName: "" + extraVolumes: [] + extraVolumeMounts: [] + + # Additional server container environment variables + # + # You specify this manually like you would a raw deployment manifest. + # This means you can bind in environment variables from secrets. + # + # e.g. static environment variable: + # - name: DEMO_GREETING + # value: "Hello from the environment" + # + # e.g. secret environment variable: + # - name: USERNAME + # valueFrom: + # secretKeyRef: + # name: mysecret + # key: username + extraEnvVars: [] + # If "installScope: cluster" then these labels will be applied to ClusterRole role: @@ -90,7 +110,7 @@ deletionPolicy: delete # controller reconciliation configurations reconcile: # The default duration, in seconds, to wait before resyncing desired state of custom resources. - defaultResyncPeriod: 0 + defaultResyncPeriod: 36000 # 10 Hours # An object representing the reconcile resync configuration for each specific resource. resourceResyncPeriods: {} diff --git a/pkg/resource/hosted_zone/delta.go b/pkg/resource/hosted_zone/delta.go index 6ce924e..eeeeadf 100644 --- a/pkg/resource/hosted_zone/delta.go +++ b/pkg/resource/hosted_zone/delta.go @@ -42,6 +42,7 @@ func newResourceDelta( delta.Add("", a, b) return delta } + compareTags(delta, a, b) if ackcompare.HasNilDifference(a.ko.Spec.DelegationSetID, b.ko.Spec.DelegationSetID) { delta.Add("Spec.DelegationSetID", a.ko.Spec.DelegationSetID, b.ko.Spec.DelegationSetID) diff --git a/pkg/resource/hosted_zone/hook.go b/pkg/resource/hosted_zone/hook.go index e684572..9a176aa 100644 --- a/pkg/resource/hosted_zone/hook.go +++ b/pkg/resource/hosted_zone/hook.go @@ -1,10 +1,15 @@ package hosted_zone import ( + "context" "fmt" "time" svcapitypes "github.com/aws-controllers-k8s/route53-controller/apis/v1alpha1" + ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" + ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" + ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log" + svcsdk "github.com/aws/aws-sdk-go/service/route53" ) // getCallerReference will generate a CallerReference for a given hosted zone @@ -13,3 +18,158 @@ import ( func getCallerReference(zone *svcapitypes.HostedZone) string { return fmt.Sprintf("%s-%d", *zone.Spec.Name, time.Now().UnixMilli()) } + +func (rm *resourceManager) customUpdateHostedZone( + ctx context.Context, + desired *resource, + latest *resource, + delta *ackcompare.Delta, +) (updated *resource, err error) { + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.customUpdateHostedZone") + defer exit(err) + + // Default `updated` to `desired` because it is likely + // EC2 `modify` APIs do NOT return output, only errors. + // If the `modify` calls (i.e. `sync`) do NOT return + // an error, then the update was successful and desired.Spec + // (now updated.Spec) reflects the latest resource state. + updated = rm.concreteResource(desired.DeepCopy()) + + if delta.DifferentAt("Spec.Tags") { + if err := rm.syncTags(ctx, desired, latest); err != nil { + return nil, err + } + } + + return updated, nil +} + +// syncTags used to keep tags in sync by calling Create and Delete API's +func (rm *resourceManager) syncTags( + ctx context.Context, + desired *resource, + latest *resource, +) (err error) { + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.syncTags") + defer func(err error) { + exit(err) + }(err) + + resourceId := latest.ko.Status.ID + + desiredTags := ToACKTags(desired.ko.Spec.Tags) + latestTags := ToACKTags(latest.ko.Spec.Tags) + + added, _, removed := ackcompare.GetTagsDifference(latestTags, desiredTags) + + toAdd := FromACKTags(added) + + var toDeleteTagKeys []*string + for k, _ := range removed { + toDeleteTagKeys = append(toDeleteTagKeys, &k) + } + + resourceType := "hostedzone" + if len(toDeleteTagKeys) > 0 { + rlog.Debug("removing tags from HostedZone resource", "tags", toDeleteTagKeys) + _, err = rm.sdkapi.ChangeTagsForResource( + &svcsdk.ChangeTagsForResourceInput{ + ResourceId: resourceId, + RemoveTagKeys: toDeleteTagKeys, + ResourceType: &resourceType, + }, + ) + rm.metrics.RecordAPICall("UPDATE", "DeleteTags", err) + if err != nil { + return err + } + + } + + if len(toAdd) > 0 { + rlog.Debug("adding tags to HostedZone resource", "tags", toAdd) + _, err = rm.sdkapi.ChangeTagsForResource( + &svcsdk.ChangeTagsForResourceInput{ + ResourceId: resourceId, + AddTags: rm.sdkTags(toAdd), + ResourceType: &resourceType, + }, + ) + rm.metrics.RecordAPICall("UPDATE", "CreateTags", err) + if err != nil { + return err + } + } + + return nil +} + +// sdkTags converts *svcapitypes.Tag array to a *svcsdk.Tag array +func (rm *resourceManager) sdkTags( + tags []*svcapitypes.Tag, +) (sdktags []*svcsdk.Tag) { + + for _, i := range tags { + sdktag := rm.newTag(*i) + sdktags = append(sdktags, sdktag) + } + + return sdktags +} + +// compareTags is a custom comparison function for comparing lists of Tag +// structs where the order of the structs in the list is not important. +func compareTags( + delta *ackcompare.Delta, + a *resource, + b *resource, +) { + if len(a.ko.Spec.Tags) != len(b.ko.Spec.Tags) { + delta.Add("Spec.Tags", a.ko.Spec.Tags, b.ko.Spec.Tags) + } else if len(a.ko.Spec.Tags) > 0 { + desiredTags := ToACKTags(a.ko.Spec.Tags) + latestTags := ToACKTags(b.ko.Spec.Tags) + + added, _, removed := ackcompare.GetTagsDifference(latestTags, desiredTags) + + toAdd := FromACKTags(added) + toDelete := FromACKTags(removed) + + if len(toAdd) != 0 || len(toDelete) != 0 { + delta.Add("Spec.Tags", a.ko.Spec.Tags, b.ko.Spec.Tags) + } + } +} + +func (rm *resourceManager) setResourceAdditionalFields( + ctx context.Context, + ko *svcapitypes.HostedZone, +) (err error) { + if ko.Status.ID != nil { + // Get the tags for hosted_zone resource + tags_input := svcsdk.ListTagsForResourceInput{} + tags_input.SetResourceId(*ko.Status.ID) + tags_input.SetResourceType("hostedzone") + var tags_resp *svcsdk.ListTagsForResourceOutput + tags_resp, err = rm.sdkapi.ListTagsForResourceWithContext(ctx, &tags_input) + rm.metrics.RecordAPICall("READ_ONE", "ListTagsForResource", err) + if err != nil { + if reqErr, ok := ackerr.AWSRequestFailure(err); ok && reqErr.StatusCode() == 404 { + return ackerr.NotFound + } + if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchHostedZone" { + return ackerr.NotFound + } + return err + } + + tags := FromRoute53Tags(tags_resp.ResourceTagSet.Tags) + + ko.Spec.Tags = tags + } else { + ko.Spec.Tags = []*svcapitypes.Tag{} + } + return nil +} diff --git a/pkg/resource/hosted_zone/manager.go b/pkg/resource/hosted_zone/manager.go index 995ceed..82dda2e 100644 --- a/pkg/resource/hosted_zone/manager.go +++ b/pkg/resource/hosted_zone/manager.go @@ -283,8 +283,20 @@ func (rm *resourceManager) EnsureTags( res acktypes.AWSResource, md acktypes.ServiceControllerMetadata, ) error { + r := rm.concreteResource(res) + if r.ko == nil { + // Should never happen... if it does, it's buggy code. + panic("resource manager's EnsureTags method received resource with nil CR object") + } + defaultTags := ackrt.GetDefaultTags(&rm.cfg, r.ko, md) + var existingTags []*svcapitypes.Tag + existingTags = r.ko.Spec.Tags + resourceTags := ToACKTags(existingTags) + tags := acktags.Merge(resourceTags, defaultTags) + r.ko.Spec.Tags = FromACKTags(tags) return nil + } // newResourceManager returns a new struct implementing diff --git a/pkg/resource/hosted_zone/sdk.go b/pkg/resource/hosted_zone/sdk.go index 3da31bb..6545205 100644 --- a/pkg/resource/hosted_zone/sdk.go +++ b/pkg/resource/hosted_zone/sdk.go @@ -136,6 +136,9 @@ func (rm *resourceManager) sdkFind( } rm.setStatusDefaults(ko) + if err := rm.setResourceAdditionalFields(ctx, ko); err != nil { + return nil, err + } return &resource{ko}, nil } @@ -243,6 +246,19 @@ func (rm *resourceManager) sdkCreate( } rm.setStatusDefaults(ko) + if ko.Status.ID != nil { + latest := &resource{} + latest.ko = &svcapitypes.HostedZone{} + latest.ko.Status.ID = ko.Status.ID + + // This is create operation. So, no tags are present in HostedZone. + // So, 'latest' is empty except we have copied 'ID' into the status to + // make syncTags() happy. + if err := rm.syncTags(ctx, desired, latest); err != nil { + return nil, err + } + } + return &resource{ko}, nil } @@ -292,7 +308,7 @@ func (rm *resourceManager) sdkUpdate( latest *resource, delta *ackcompare.Delta, ) (*resource, error) { - return nil, ackerr.NewTerminalError(ackerr.NotImplemented) + return rm.customUpdateHostedZone(ctx, desired, latest, delta) } // sdkDelete deletes the supplied resource in the backend AWS service API @@ -446,3 +462,17 @@ func (rm *resourceManager) terminalAWSError(err error) bool { return false } } + +func (rm *resourceManager) newTag( + c svcapitypes.Tag, +) *svcsdk.Tag { + res := &svcsdk.Tag{} + if c.Key != nil { + res.SetKey(*c.Key) + } + if c.Value != nil { + res.SetValue(*c.Value) + } + + return res +} diff --git a/pkg/resource/hosted_zone/tags.go b/pkg/resource/hosted_zone/tags.go new file mode 100644 index 0000000..492ec57 --- /dev/null +++ b/pkg/resource/hosted_zone/tags.go @@ -0,0 +1,76 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +// Code generated by ack-generate. DO NOT EDIT. + +package hosted_zone + +import ( + acktags "github.com/aws-controllers-k8s/runtime/pkg/tags" + + svcapitypes "github.com/aws-controllers-k8s/route53-controller/apis/v1alpha1" + svcsdk "github.com/aws/aws-sdk-go/service/route53" +) + +var ( + _ = svcapitypes.HostedZone{} + _ = acktags.NewTags() +) + +// ToACKTags converts the tags parameter into 'acktags.Tags' shape. +// This method helps in creating the hub(acktags.Tags) for merging +// default controller tags with existing resource tags. +func ToACKTags(tags []*svcapitypes.Tag) acktags.Tags { + result := acktags.NewTags() + if tags == nil || len(tags) == 0 { + return result + } + + for _, t := range tags { + if t.Key != nil { + if t.Value == nil { + result[*t.Key] = "" + } else { + result[*t.Key] = *t.Value + } + } + } + + return result +} + +// FromACKTags converts the tags parameter into []*svcapitypes.Tag shape. +// This method helps in setting the tags back inside AWSResource after merging +// default controller tags with existing resource tags. +func FromACKTags(tags acktags.Tags) []*svcapitypes.Tag { + result := []*svcapitypes.Tag{} + for k, v := range tags { + kCopy := k + vCopy := v + tag := svcapitypes.Tag{Key: &kCopy, Value: &vCopy} + result = append(result, &tag) + } + return result +} + +// FromRoute53Tags converts the tags parameter into []*svcapitypes.Tag shape. +func FromRoute53Tags(tags []*svcsdk.Tag) []*svcapitypes.Tag { + result := []*svcapitypes.Tag{} + for _, tag := range tags { + kCopy := *tag.Key + vCopy := *tag.Value + svcapiTag := svcapitypes.Tag{Key: &kCopy, Value: &vCopy} + result = append(result, &svcapiTag) + } + return result +} diff --git a/templates/hooks/hosted_zone/sdk_create_post_set_output.go.tpl b/templates/hooks/hosted_zone/sdk_create_post_set_output.go.tpl new file mode 100644 index 0000000..ae2dbe2 --- /dev/null +++ b/templates/hooks/hosted_zone/sdk_create_post_set_output.go.tpl @@ -0,0 +1,12 @@ + if ko.Status.ID != nil { + latest := &resource{} + latest.ko = &svcapitypes.HostedZone{} + latest.ko.Status.ID = ko.Status.ID + + // This is create operation. So, no tags are present in HostedZone. + // So, 'latest' is empty except we have copied 'ID' into the status to + // make syncTags() happy. + if err := rm.syncTags(ctx, desired, latest); err != nil { + return nil, err + } + } diff --git a/templates/hooks/hosted_zone/sdk_ensure_tags.go.tpl b/templates/hooks/hosted_zone/sdk_ensure_tags.go.tpl new file mode 100644 index 0000000..bc658a0 --- /dev/null +++ b/templates/hooks/hosted_zone/sdk_ensure_tags.go.tpl @@ -0,0 +1,13 @@ + r := rm.concreteResource(res) + if r.ko == nil { + // Should never happen... if it does, it's buggy code. + panic("resource manager's EnsureTags method received resource with nil CR object") + } + defaultTags := ackrt.GetDefaultTags(&rm.cfg, r.ko, md) + var existingTags []*svcapitypes.Tag + existingTags = r.ko.Spec.Tags + + resourceTags := ToACKTags(existingTags) + tags := acktags.Merge(resourceTags, defaultTags) + r.ko.Spec.Tags = FromACKTags(tags) + return nil \ No newline at end of file diff --git a/templates/hooks/hosted_zone/sdk_file_end.go.tpl b/templates/hooks/hosted_zone/sdk_file_end.go.tpl new file mode 100644 index 0000000..66eb8b5 --- /dev/null +++ b/templates/hooks/hosted_zone/sdk_file_end.go.tpl @@ -0,0 +1,23 @@ +{{ $CRD := .CRD }} +{{ $SDKAPI := .SDKAPI }} + +{{/* Generate helper methods for HostedZone */}} +{{- range $specFieldName, $specField := $CRD.Config.Resources.HostedZone.Fields }} +{{- if $specField.From }} +{{- $operationName := $specField.From.Operation }} +{{- $operation := (index $SDKAPI.API.Operations $operationName) -}} +{{- range $hostedZoneRefName, $hostedZoneMemberRefs := $operation.InputRef.Shape.MemberRefs -}} +{{- if eq $hostedZoneRefName "AddTags" }} +{{- $hostedZoneRef := $hostedZoneMemberRefs.Shape.MemberRef }} +{{- $hostedZoneRefName = "Tag" }} +func (rm *resourceManager) new{{ $hostedZoneRefName }}( + c svcapitypes.{{ $hostedZoneRefName }}, +) *svcsdk.{{ $hostedZoneRefName }} { + res := &svcsdk.{{ $hostedZoneRefName }}{} +{{ GoCodeSetSDKForStruct $CRD "" "res" $hostedZoneRef "" "c" 1 }} + return res +} +{{- end }} +{{- end }} +{{- end }} +{{- end }} \ No newline at end of file diff --git a/templates/hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl b/templates/hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl new file mode 100644 index 0000000..4e8ce2f --- /dev/null +++ b/templates/hooks/hosted_zone/sdk_read_one_post_set_output.go.tpl @@ -0,0 +1,3 @@ + if err := rm.setResourceAdditionalFields(ctx, ko); err != nil { + return nil, err + } \ No newline at end of file diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py index 5c974b6..78cde04 100644 --- a/test/e2e/conftest.py +++ b/test/e2e/conftest.py @@ -32,6 +32,9 @@ def pytest_configure(config): config.addinivalue_line( "markers", "slow: mark test as slow to run" ) + config.addinivalue_line( + "markers", "resource_data: mark test with data to use when creating fixture" + ) def pytest_collection_modifyitems(config, items): if config.getoption("--runslow"): diff --git a/test/e2e/requirements.txt b/test/e2e/requirements.txt index 97ece3e..8d94056 100644 --- a/test/e2e/requirements.txt +++ b/test/e2e/requirements.txt @@ -1 +1 @@ -acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@835781dcbb39e2220e68a659dd771ce4bd9b1ac0 +acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@38ce32256cc2552ab54e190cc8a8618e93af9e0c diff --git a/test/e2e/resources/hosted_zone_public.yaml b/test/e2e/resources/hosted_zone_public.yaml index ea1aea1..60fc503 100644 --- a/test/e2e/resources/hosted_zone_public.yaml +++ b/test/e2e/resources/hosted_zone_public.yaml @@ -4,3 +4,6 @@ metadata: name: $ZONE_NAME spec: name: $ZONE_DOMAIN + tags: + - key: $TAG_KEY + value: $TAG_VALUE diff --git a/test/e2e/tests/helper.py b/test/e2e/tests/helper.py new file mode 100644 index 0000000..d645c7d --- /dev/null +++ b/test/e2e/tests/helper.py @@ -0,0 +1,46 @@ +# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You may +# not use this file except in compliance with the License. A copy of the +# License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is distributed +# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +# express or implied. See the License for the specific language governing +# permissions and limitations under the License. + +"""Helper functions for route53 tests +""" + +import pytest + +from typing import Union, Dict + + +class Route53Validator: + def __init__(self, route53_client): + self.route53_client = route53_client + + def list_tags_for_resources(self, resource_id: str, resource_type: str): + resource_id = resource_id.replace('/' + resource_type + '/', '') + try: + aws_res = self.route53_client.list_tags_for_resource( + ResourceType=resource_type, + ResourceId=resource_id) + assert aws_res is not None + if len(aws_res["ResourceTagSet"]) > 0: + return aws_res["ResourceTagSet"] + assert False + except self.route53_client.exceptions.ClientError as e: + return None + + def assert_hosted_zone(self, hosted_zone_id: str, exists=True): + res_found = False + try: + aws_res = self.route53_client.get_hosted_zone(Id=hosted_zone_id) + res_found = len(aws_res["HostedZone"]) > 0 + except self.route53_client.exceptions.ClientError: + pass + assert res_found is exists diff --git a/test/e2e/tests/test_hosted_zone.py b/test/e2e/tests/test_hosted_zone.py index d146962..c7d9410 100644 --- a/test/e2e/tests/test_hosted_zone.py +++ b/test/e2e/tests/test_hosted_zone.py @@ -21,29 +21,42 @@ import pytest +from acktest import tags from acktest.k8s import resource as k8s from acktest.k8s import condition from acktest.resources import random_suffix_name from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_eks_resource from e2e.replacement_values import REPLACEMENT_VALUES from e2e.bootstrap_resources import get_bootstrap_resources +from e2e.tests.helper import Route53Validator RESOURCE_PLURAL = "hostedzones" # Time to wait after modifying the CR for the status to change MODIFY_WAIT_AFTER_SECONDS = 10 +CREATE_WAIT_AFTER_SECONDS = 10 +DELETE_WAIT_AFTER_SECONDS = 10 + # Time to wait after the zone has changed status, for the CR to update CHECK_STATUS_WAIT_SECONDS = 10 @pytest.fixture -def public_hosted_zone(): +def public_hosted_zone(request): zone_name = random_suffix_name("public-hosted-zone", 32) replacements = REPLACEMENT_VALUES.copy() replacements["ZONE_NAME"] = zone_name replacements["ZONE_DOMAIN"] = f"{zone_name}.ack.example.com." + marker = request.node.get_closest_marker("resource_data") + if marker is not None: + data = marker.args[0] + if 'tag_key' in data: + replacements["TAG_KEY"] = data['tag_key'] + if 'tag_value' in data: + replacements["TAG_VALUE"] = data['tag_value'] + resource_data = load_eks_resource( "hosted_zone_public", additional_replacements=replacements, @@ -108,6 +121,7 @@ def private_hosted_zone(): @service_marker @pytest.mark.canary class TestHostedZone: + @pytest.mark.resource_data({'tag_key': 'key', 'tag_value': 'value'}) def test_create_delete_public(self, route53_client, public_hosted_zone): (ref, cr) = public_hosted_zone @@ -115,11 +129,9 @@ def test_create_delete_public(self, route53_client, public_hosted_zone): assert zone_id - try: - aws_res = route53_client.get_hosted_zone(Id=zone_id) - assert aws_res is not None - except route53_client.exceptions.NoSuchHostedZone: - pytest.fail(f"Could not find hosted zone with ID '{zone_id}' in Route53") + # Check hosted_zone exists in AWS + route53_validator = Route53Validator(route53_client) + route53_validator.assert_hosted_zone(zone_id) def test_create_delete_private(self, route53_client, private_hosted_zone): (ref, cr) = private_hosted_zone @@ -128,8 +140,109 @@ def test_create_delete_private(self, route53_client, private_hosted_zone): assert zone_id - try: - aws_res = route53_client.get_hosted_zone(Id=zone_id) - assert aws_res is not None - except route53_client.exceptions.NoSuchHostedZone: - pytest.fail(f"Could not find hosted zone with ID '{zone_id}' in Route53") \ No newline at end of file + # Check hosted_zone exists in AWS + route53_validator = Route53Validator(route53_client) + route53_validator.assert_hosted_zone(zone_id) + + @pytest.mark.resource_data({'tag_key': 'initialtagkey', 'tag_value': 'initialtagvalue'}) + def test_crud_tags(self, route53_client, public_hosted_zone): + (ref, cr) = public_hosted_zone + + resource = k8s.get_resource(ref) + resource_id = cr["status"]["id"] + + time.sleep(CREATE_WAIT_AFTER_SECONDS) + + # Check hosted_zone exists in AWS + route53_validator = Route53Validator(route53_client) + route53_validator.assert_hosted_zone(resource_id) + + # Check system and user tags exist for hosted_zone resource + hosted_zone = route53_validator.list_tags_for_resources(resource_id, "hostedzone") + user_tags = { + "initialtagkey": "initialtagvalue" + } + tags.assert_ack_system_tags( + tags=hosted_zone["Tags"], + ) + tags.assert_equal_without_ack_tags( + expected=user_tags, + actual=hosted_zone["Tags"], + ) + + # Only user tags should be present in Spec + assert len(resource["spec"]["tags"]) == 1 + assert resource["spec"]["tags"][0]["key"] == "initialtagkey" + assert resource["spec"]["tags"][0]["value"] == "initialtagvalue" + + # Update tags + update_tags = [ + { + "key": "updatedtagkey", + "value": "updatedtagvalue", + } + ] + + # Patch the dhcpOptions, updating the tags with new pair + updates = { + "spec": {"tags": update_tags}, + } + + k8s.patch_custom_resource(ref, updates) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + + # Check resource synced successfully + assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5) + + # Check for updated user tags; system tags should persist + hosted_zone = route53_validator.list_tags_for_resources(resource_id, "hostedzone") + updated_tags = { + "updatedtagkey": "updatedtagvalue" + } + tags.assert_ack_system_tags( + tags=hosted_zone["Tags"], + ) + tags.assert_equal_without_ack_tags( + expected=updated_tags, + actual=hosted_zone["Tags"], + ) + + # Only user tags should be present in Spec + resource = k8s.get_resource(ref) + assert len(resource["spec"]["tags"]) == 1 + assert resource["spec"]["tags"][0]["key"] == "updatedtagkey" + assert resource["spec"]["tags"][0]["value"] == "updatedtagvalue" + + # Patch the dhcpOptions resource, deleting the tags + updates = { + "spec": {"tags": []}, + } + + k8s.patch_custom_resource(ref, updates) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + + # Check resource synced successfully + assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5) + + # Check for removed user tags; system tags should persist + hosted_zone = route53_validator.list_tags_for_resources(resource_id, "hostedzone") + tags.assert_ack_system_tags( + tags=hosted_zone["Tags"], + ) + tags.assert_equal_without_ack_tags( + expected=[], + actual=hosted_zone["Tags"], + ) + + # Check user tags are removed from Spec + resource = k8s.get_resource(ref) + assert len(resource["spec"]["tags"]) == 0 + + # Delete k8s resource + _, deleted = k8s.delete_custom_resource(ref) + assert deleted is True + + time.sleep(DELETE_WAIT_AFTER_SECONDS) + + # Check hosted_zone no longer exists in AWS + route53_validator.assert_hosted_zone(resource_id, exists=False)