From 50ab82ed04027ab37d314696c8f780531ba5bc4f Mon Sep 17 00:00:00 2001 From: AkiraFukushima Date: Sat, 17 Jul 2021 11:42:47 +0900 Subject: [PATCH 1/4] Support AWS LB access log configuration in cluster spec --- pkg/apis/kops/cluster.go | 11 ++++ pkg/apis/kops/v1alpha2/cluster.go | 11 ++++ .../kops/v1alpha2/zz_generated.conversion.go | 52 +++++++++++++++++++ .../kops/v1alpha2/zz_generated.deepcopy.go | 21 ++++++++ pkg/apis/kops/zz_generated.deepcopy.go | 21 ++++++++ pkg/model/awsmodel/api_loadbalancer.go | 9 ++++ .../classic_loadbalancer_attributes.go | 3 ++ 7 files changed, 128 insertions(+) diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 93572e939b..dee7fa1d0a 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -411,6 +411,15 @@ const ( LoadBalancerClassNetwork LoadBalancerClass = "Network" ) +type AccessLogSpec struct { + // Interval is publishing interval in minutes + Interval int `json:"interval,omitempty"` + // Bucket is S3 bucket name to store the logs in + Bucket string `json:"bucket,omitempty"` + // BucketPrefix is S3 bucket prefix. Logs are stored in the root if not configured. + BucketPrefix string `json:"bucketPrefix,omitempty"` +} + var SupportedLoadBalancerClasses = []string{ string(LoadBalancerClassClassic), string(LoadBalancerClassNetwork), @@ -448,6 +457,8 @@ type LoadBalancerAccessSpec struct { CrossZoneLoadBalancing *bool `json:"crossZoneLoadBalancing,omitempty"` // Subnets allows you to specify the subnets that must be used for the load balancer Subnets []LoadBalancerSubnetSpec `json:"subnets,omitempty"` + // AccessLog is the configuration of access logs + AccessLog *AccessLogSpec `json:"accessLog,omitempty"` } // KubeDNSConfig defines the kube dns configuration diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index fdb0376bcc..35c52fee99 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/pkg/apis/kops/v1alpha2/cluster.go @@ -412,6 +412,15 @@ const ( LoadBalancerClassNetwork LoadBalancerClass = "Network" ) +type AccessLogSpec struct { + // Interval is publishing interval in minutes + Interval int `json:"interval,omitempty"` + // Bucket is S3 bucket name to store the logs in + Bucket string `json:"bucket,omitempty"` + // BucketPrefix is S3 bucket prefix. Logs are stored in the root if not configured. + BucketPrefix string `json:"bucketPrefix,omitempty"` +} + var SupportedLoadBalancerClasses = []string{ string(LoadBalancerClassClassic), string(LoadBalancerClassNetwork), @@ -449,6 +458,8 @@ type LoadBalancerAccessSpec struct { CrossZoneLoadBalancing *bool `json:"crossZoneLoadBalancing,omitempty"` // Subnets allows you to specify the subnets that must be used for the load balancer Subnets []LoadBalancerSubnetSpec `json:"subnets,omitempty"` + // AccessLog is the configuration of access logs + AccessLog *AccessLogSpec `json:"accessLog,omitempty"` } // KubeDNSConfig defines the kube dns configuration diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index 06ffd87afa..06da9370f9 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go @@ -63,6 +63,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*AccessLogSpec)(nil), (*kops.AccessLogSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec(a.(*AccessLogSpec), b.(*kops.AccessLogSpec), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*kops.AccessLogSpec)(nil), (*AccessLogSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec(a.(*kops.AccessLogSpec), b.(*AccessLogSpec), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*AccessSpec)(nil), (*kops.AccessSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha2_AccessSpec_To_kops_AccessSpec(a.(*AccessSpec), b.(*kops.AccessSpec), scope) }); err != nil { @@ -1184,6 +1194,30 @@ func Convert_kops_AWSPermission_To_v1alpha2_AWSPermission(in *kops.AWSPermission return autoConvert_kops_AWSPermission_To_v1alpha2_AWSPermission(in, out, s) } +func autoConvert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec(in *AccessLogSpec, out *kops.AccessLogSpec, s conversion.Scope) error { + out.Interval = in.Interval + out.Bucket = in.Bucket + out.BucketPrefix = in.BucketPrefix + return nil +} + +// Convert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec is an autogenerated conversion function. +func Convert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec(in *AccessLogSpec, out *kops.AccessLogSpec, s conversion.Scope) error { + return autoConvert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec(in, out, s) +} + +func autoConvert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec(in *kops.AccessLogSpec, out *AccessLogSpec, s conversion.Scope) error { + out.Interval = in.Interval + out.Bucket = in.Bucket + out.BucketPrefix = in.BucketPrefix + return nil +} + +// Convert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec is an autogenerated conversion function. +func Convert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec(in *kops.AccessLogSpec, out *AccessLogSpec, s conversion.Scope) error { + return autoConvert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec(in, out, s) +} + func autoConvert_v1alpha2_AccessSpec_To_kops_AccessSpec(in *AccessSpec, out *kops.AccessSpec, s conversion.Scope) error { if in.DNS != nil { in, out := &in.DNS, &out.DNS @@ -5441,6 +5475,15 @@ func autoConvert_v1alpha2_LoadBalancerAccessSpec_To_kops_LoadBalancerAccessSpec( } else { out.Subnets = nil } + if in.AccessLog != nil { + in, out := &in.AccessLog, &out.AccessLog + *out = new(kops.AccessLogSpec) + if err := Convert_v1alpha2_AccessLogSpec_To_kops_AccessLogSpec(*in, *out, s); err != nil { + return err + } + } else { + out.AccessLog = nil + } return nil } @@ -5470,6 +5513,15 @@ func autoConvert_kops_LoadBalancerAccessSpec_To_v1alpha2_LoadBalancerAccessSpec( } else { out.Subnets = nil } + if in.AccessLog != nil { + in, out := &in.AccessLog, &out.AccessLog + *out = new(AccessLogSpec) + if err := Convert_kops_AccessLogSpec_To_v1alpha2_AccessLogSpec(*in, *out, s); err != nil { + return err + } + } else { + out.AccessLog = nil + } return nil } diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go index 5c06cd0916..7d3b93233a 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go @@ -104,6 +104,22 @@ func (in *AWSPermission) DeepCopy() *AWSPermission { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogSpec) DeepCopyInto(out *AccessLogSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogSpec. +func (in *AccessLogSpec) DeepCopy() *AccessLogSpec { + if in == nil { + return nil + } + out := new(AccessLogSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccessSpec) DeepCopyInto(out *AccessSpec) { *out = *in @@ -3639,6 +3655,11 @@ func (in *LoadBalancerAccessSpec) DeepCopyInto(out *LoadBalancerAccessSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.AccessLog != nil { + in, out := &in.AccessLog, &out.AccessLog + *out = new(AccessLogSpec) + **out = **in + } return } diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go index fdf27bb8a7..024b143d4f 100644 --- a/pkg/apis/kops/zz_generated.deepcopy.go +++ b/pkg/apis/kops/zz_generated.deepcopy.go @@ -104,6 +104,22 @@ func (in *AWSPermission) DeepCopy() *AWSPermission { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogSpec) DeepCopyInto(out *AccessLogSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogSpec. +func (in *AccessLogSpec) DeepCopy() *AccessLogSpec { + if in == nil { + return nil + } + out := new(AccessLogSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccessSpec) DeepCopyInto(out *AccessSpec) { *out = *in @@ -3821,6 +3837,11 @@ func (in *LoadBalancerAccessSpec) DeepCopyInto(out *LoadBalancerAccessSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.AccessLog != nil { + in, out := &in.AccessLog, &out.AccessLog + *out = new(AccessLogSpec) + **out = **in + } return } diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 42fbd7e5db..146fc14706 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -237,6 +237,15 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { return fmt.Errorf("unknown load balancer Type: %q", lbSpec.Type) } + if lbSpec.AccessLog != nil { + clb.AccessLog = &awstasks.ClassicLoadBalancerAccessLog{ + EmitInterval: fi.Int64(int64(lbSpec.AccessLog.Interval)), + Enabled: fi.Bool(true), + S3BucketName: fi.String(lbSpec.AccessLog.Bucket), + S3BucketPrefix: fi.String(lbSpec.AccessLog.BucketPrefix), + } + } + if b.APILoadBalancerClass() == kops.LoadBalancerClassClassic { c.AddTask(clb) } else if b.APILoadBalancerClass() == kops.LoadBalancerClassNetwork { diff --git a/upup/pkg/fi/cloudup/awstasks/classic_loadbalancer_attributes.go b/upup/pkg/fi/cloudup/awstasks/classic_loadbalancer_attributes.go index cb893b6f51..2c1ec749c9 100644 --- a/upup/pkg/fi/cloudup/awstasks/classic_loadbalancer_attributes.go +++ b/upup/pkg/fi/cloudup/awstasks/classic_loadbalancer_attributes.go @@ -153,6 +153,9 @@ func (_ *ClassicLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget // request.LoadBalancerAttributes.AdditionalAttributes = additionalAttributes //} + if e.AccessLog != nil && e.AccessLog.Enabled != nil { + request.LoadBalancerAttributes.AccessLog.Enabled = e.AccessLog.Enabled + } if e.AccessLog != nil && e.AccessLog.EmitInterval != nil { request.LoadBalancerAttributes.AccessLog.EmitInterval = e.AccessLog.EmitInterval } From 226cbe5561d1909fc4ae96039931fbea0dac8774 Mon Sep 17 00:00:00 2001 From: AkiraFukushima Date: Thu, 29 Jul 2021 22:47:42 +0900 Subject: [PATCH 2/4] Support AWS LB access log configuration for NetworkLoadBalancer --- k8s/crds/kops.k8s.io_clusters.yaml | 16 ++++++++ pkg/apis/kops/cluster.go | 8 ++-- pkg/apis/kops/v1alpha2/cluster.go | 2 +- pkg/model/awsmodel/api_loadbalancer.go | 5 +++ .../cloudup/awstasks/network_load_balancer.go | 40 +++++++++++++++++++ .../networkloadbalancer_attributes.go | 32 +++++++++++++++ 6 files changed, 98 insertions(+), 5 deletions(-) diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index 24e1e461bb..d50a766e7b 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -81,6 +81,22 @@ spec: description: LoadBalancer is the configuration for the kube-apiserver ELB properties: + accessLog: + description: AccessLog is the configuration of access logs + properties: + bucket: + description: Bucket is S3 bucket name to store the logs + in + type: string + bucketPrefix: + description: BucketPrefix is S3 bucket prefix. Logs are + stored in the root if not configured. + type: string + interval: + description: Interval is publishing interval in minutes. + This parameter is only used with classic load balancer. + type: integer + type: object additionalSecurityGroups: description: AdditionalSecurityGroups attaches additional security groups (e.g. sg-123456). diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index dee7fa1d0a..5187972605 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -412,11 +412,11 @@ const ( ) type AccessLogSpec struct { - // Interval is publishing interval in minutes + // Interval is the publishing interval in minutes. This parameter is only used with classic load balancer. Interval int `json:"interval,omitempty"` - // Bucket is S3 bucket name to store the logs in + // Bucket is the S3 bucket name to store the logs in. Bucket string `json:"bucket,omitempty"` - // BucketPrefix is S3 bucket prefix. Logs are stored in the root if not configured. + // BucketPrefix is the S3 bucket prefix. Logs are stored in the root if not configured. BucketPrefix string `json:"bucketPrefix,omitempty"` } @@ -457,7 +457,7 @@ type LoadBalancerAccessSpec struct { CrossZoneLoadBalancing *bool `json:"crossZoneLoadBalancing,omitempty"` // Subnets allows you to specify the subnets that must be used for the load balancer Subnets []LoadBalancerSubnetSpec `json:"subnets,omitempty"` - // AccessLog is the configuration of access logs + // AccessLog is the configuration of access logs. AccessLog *AccessLogSpec `json:"accessLog,omitempty"` } diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index 35c52fee99..077e35f156 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/pkg/apis/kops/v1alpha2/cluster.go @@ -413,7 +413,7 @@ const ( ) type AccessLogSpec struct { - // Interval is publishing interval in minutes + // Interval is publishing interval in minutes. This parameter is only used with classic load balancer. Interval int `json:"interval,omitempty"` // Bucket is S3 bucket name to store the logs in Bucket string `json:"bucket,omitempty"` diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 146fc14706..5ef9c05332 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -244,6 +244,11 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { S3BucketName: fi.String(lbSpec.AccessLog.Bucket), S3BucketPrefix: fi.String(lbSpec.AccessLog.BucketPrefix), } + nlb.AccessLog = &awstasks.NetworkLoadBalancerAccessLog{ + Enabled: fi.Bool(true), + S3BucketName: fi.String(lbSpec.AccessLog.Bucket), + S3BucketPrefix: fi.String(lbSpec.AccessLog.BucketPrefix), + } } if b.APILoadBalancerClass() == kops.LoadBalancerClassClassic { diff --git a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go index fdfbc2ffc7..60bb3b6856 100644 --- a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go @@ -69,6 +69,7 @@ type NetworkLoadBalancer struct { VPC *VPC TargetGroups []*TargetGroup + AccessLog *NetworkLoadBalancerAccessLog } var _ fi.CompareWithID = &NetworkLoadBalancer{} @@ -365,6 +366,34 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error) return nil, err } actual.CrossZoneLoadBalancing = fi.Bool(b) + case "access_logs.s3.enabled": + b, err := strconv.ParseBool(*value) + if err != nil { + return nil, err + } + if actual.AccessLog != nil { + actual.AccessLog.Enabled = fi.Bool(b) + } else { + actual.AccessLog = &NetworkLoadBalancerAccessLog{ + Enabled: fi.Bool(b), + } + } + case "access_logs.s3.bucket": + if actual.AccessLog != nil { + actual.AccessLog.S3BucketName = value + } else { + actual.AccessLog = &NetworkLoadBalancerAccessLog{ + S3BucketName: value, + } + } + case "access_logs.s3.prefix": + if actual.AccessLog != nil { + actual.AccessLog.S3BucketPrefix = value + } else { + actual.AccessLog = &NetworkLoadBalancerAccessLog{ + S3BucketPrefix: value, + } + } default: klog.V(2).Infof("unsupported key -- ignoring, %v.\n", key) } @@ -454,6 +483,17 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e return fi.RequiredField("CrossZoneLoadBalancing") } } + + if e.AccessLog != nil { + if e.AccessLog.Enabled == nil { + return fi.RequiredField("Accesslog.Enabled") + } + if *e.AccessLog.Enabled { + if e.AccessLog.S3BucketName == nil { + return fi.RequiredField("Accesslog.S3Bucket") + } + } + } } else { if len(changes.SubnetMappings) > 0 { expectedSubnets := make(map[string]*string) diff --git a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go index 761c76d8b6..be628211f7 100644 --- a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go +++ b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go @@ -27,6 +27,16 @@ import ( "k8s.io/kops/upup/pkg/fi/cloudup/awsup" ) +type NetworkLoadBalancerAccessLog struct { + Enabled *bool + S3BucketName *string + S3BucketPrefix *string +} + +func (_ *NetworkLoadBalancerAccessLog) GetDependencies(tasks map[string]fi.Task) []fi.Task { + return nil +} + func findNetworkLoadBalancerAttributes(cloud awsup.AWSCloud, LoadBalancerArn string) ([]*elbv2.LoadBalancerAttribute, error) { request := &elbv2.DescribeLoadBalancerAttributesInput{ @@ -74,6 +84,28 @@ func (_ *NetworkLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget } attributes = append(attributes, attribute) + if e.AccessLog != nil && e.AccessLog.Enabled != nil { + attr := &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.enabled"), + Value: aws.String(strconv.FormatBool(aws.BoolValue(e.AccessLog.Enabled))), + } + attributes = append(attributes, attr) + } + if e.AccessLog != nil && e.AccessLog.S3BucketName != nil { + attr := &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.bucket"), + Value: e.AccessLog.S3BucketName, + } + attributes = append(attributes, attr) + } + if e.AccessLog != nil && e.AccessLog.S3BucketPrefix != nil { + attr := &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.prefix"), + Value: e.AccessLog.S3BucketPrefix, + } + attributes = append(attributes, attr) + } + request.Attributes = attributes klog.V(2).Infof("Configuring NLB attributes for NLB %q", loadBalancerName) From 2fd69ba3a33f6a82f50f4436a8052119f05ec633 Mon Sep 17 00:00:00 2001 From: AkiraFukushima Date: Tue, 3 Aug 2021 17:45:20 +0900 Subject: [PATCH 3/4] Remove access log attributes when the spec is removed from cluster spec --- pkg/model/awsmodel/api_loadbalancer.go | 7 +++++++ .../fi/cloudup/awstasks/networkloadbalancer_attributes.go | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 5ef9c05332..808ee3bcfd 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -249,6 +249,13 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { S3BucketName: fi.String(lbSpec.AccessLog.Bucket), S3BucketPrefix: fi.String(lbSpec.AccessLog.BucketPrefix), } + } else { + clb.AccessLog = &awstasks.ClassicLoadBalancerAccessLog{ + Enabled: fi.Bool(false), + } + nlb.AccessLog = &awstasks.NetworkLoadBalancerAccessLog{ + Enabled: fi.Bool(false), + } } if b.APILoadBalancerClass() == kops.LoadBalancerClassClassic { diff --git a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go index be628211f7..3c307967f2 100644 --- a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go +++ b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go @@ -62,7 +62,7 @@ func findNetworkLoadBalancerAttributes(cloud awsup.AWSCloud, LoadBalancerArn str } func (_ *NetworkLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget, a, e, changes *NetworkLoadBalancer, loadBalancerArn string) error { - if changes.CrossZoneLoadBalancing == nil { + if changes.CrossZoneLoadBalancing == nil && changes.AccessLog == nil { klog.V(4).Infof("No LoadBalancerAttribute changes; skipping update") return nil } @@ -84,7 +84,7 @@ func (_ *NetworkLoadBalancer) modifyLoadBalancerAttributes(t *awsup.AWSAPITarget } attributes = append(attributes, attribute) - if e.AccessLog != nil && e.AccessLog.Enabled != nil { + if e.AccessLog != nil { attr := &elbv2.LoadBalancerAttribute{ Key: aws.String("access_logs.s3.enabled"), Value: aws.String(strconv.FormatBool(aws.BoolValue(e.AccessLog.Enabled))), From 73f7307844a98a591fea74899e3e2684f3d3a076 Mon Sep 17 00:00:00 2001 From: AkiraFukushima Date: Tue, 3 Aug 2021 21:34:03 +0900 Subject: [PATCH 4/4] Add AccessLog attribute to CloudFormation and Terraform renderer --- .../complex/cloudformation.json | 14 ++++ ...cket_object_cluster-completed.spec_content | 2 + .../complex/in-legacy-v1alpha2.yaml | 2 + .../update_cluster/complex/in-v1alpha2.yaml | 2 + .../update_cluster/complex/kubernetes.tf | 5 ++ .../cloudup/awstasks/classic_load_balancer.go | 4 +- .../cloudup/awstasks/network_load_balancer.go | 72 +++++++++++++------ .../networkloadbalancer_attributes.go | 6 ++ 8 files changed, 82 insertions(+), 25 deletions(-) diff --git a/tests/integration/update_cluster/complex/cloudformation.json b/tests/integration/update_cluster/complex/cloudformation.json index f6c193d00a..e21488f4f1 100644 --- a/tests/integration/update_cluster/complex/cloudformation.json +++ b/tests/integration/update_cluster/complex/cloudformation.json @@ -1458,6 +1458,20 @@ "Key": "kubernetes.io/cluster/complex.example.com", "Value": "owned" } + ], + "LoadBalancerAttributes": [ + { + "Key": "access_logs.s3.enabled", + "Value": "true" + }, + { + "Key": "access_logs.s3.bucket", + "Value": "access-log-example" + }, + { + "Key": "access_logs.s3.prefix", + "Value": "" + } ] } }, diff --git a/tests/integration/update_cluster/complex/data/aws_s3_bucket_object_cluster-completed.spec_content b/tests/integration/update_cluster/complex/data/aws_s3_bucket_object_cluster-completed.spec_content index 6a2f46b751..3a7c44b026 100644 --- a/tests/integration/update_cluster/complex/data/aws_s3_bucket_object_cluster-completed.spec_content +++ b/tests/integration/update_cluster/complex/data/aws_s3_bucket_object_cluster-completed.spec_content @@ -9,6 +9,8 @@ spec: - 10.2.0.0/16 api: loadBalancer: + accessLog: + bucket: access-log-example additionalSecurityGroups: - sg-exampleid5 - sg-exampleid6 diff --git a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml index 49240f4da0..1d48c29daf 100644 --- a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml @@ -17,6 +17,8 @@ spec: subnets: - name: us-test-1a allocationId: eipalloc-012345a678b9cdefa + accessLog: + bucket: access-log-example kubernetesApiAccess: - 1.1.1.0/24 channel: stable diff --git a/tests/integration/update_cluster/complex/in-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-v1alpha2.yaml index 882f42d99f..ca5b723431 100644 --- a/tests/integration/update_cluster/complex/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-v1alpha2.yaml @@ -17,6 +17,8 @@ spec: subnets: - name: us-test-1a allocationId: eipalloc-012345a678b9cdefa + accessLog: + bucket: access-log-example kubernetesApiAccess: - 1.1.1.0/24 channel: stable diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index dc51cce5db..9f08f2141f 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/tests/integration/update_cluster/complex/kubernetes.tf @@ -523,6 +523,11 @@ resource "aws_launch_template" "nodes-complex-example-com" { } resource "aws_lb" "api-complex-example-com" { + access_logs { + bucket = "access-log-example" + enabled = true + prefix = "" + } enable_cross_zone_load_balancing = true internal = false load_balancer_type = "network" diff --git a/upup/pkg/fi/cloudup/awstasks/classic_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/classic_load_balancer.go index 4f2ee6fccf..08941599ac 100644 --- a/upup/pkg/fi/cloudup/awstasks/classic_load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/classic_load_balancer.go @@ -716,7 +716,7 @@ func (_ *ClassicLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e } } - if e.AccessLog != nil { + if e.AccessLog != nil && fi.BoolValue(e.AccessLog.Enabled) { tf.AccessLog = &terraformLoadBalancerAccessLog{ EmitInterval: e.AccessLog.EmitInterval, Enabled: e.AccessLog.Enabled, @@ -856,7 +856,7 @@ func (_ *ClassicLoadBalancer) RenderCloudformation(t *cloudformation.Cloudformat } } - if e.AccessLog != nil { + if e.AccessLog != nil && fi.BoolValue(e.AccessLog.Enabled) { tf.AccessLog = &cloudformationClassicLoadBalancerAccessLog{ EmitInterval: e.AccessLog.EmitInterval, Enabled: e.AccessLog.Enabled, diff --git a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go index 60bb3b6856..5058a225f1 100644 --- a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go @@ -371,29 +371,20 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error) if err != nil { return nil, err } - if actual.AccessLog != nil { - actual.AccessLog.Enabled = fi.Bool(b) - } else { - actual.AccessLog = &NetworkLoadBalancerAccessLog{ - Enabled: fi.Bool(b), - } + if actual.AccessLog == nil { + actual.AccessLog = &NetworkLoadBalancerAccessLog{} } + actual.AccessLog.Enabled = fi.Bool(b) case "access_logs.s3.bucket": - if actual.AccessLog != nil { - actual.AccessLog.S3BucketName = value - } else { - actual.AccessLog = &NetworkLoadBalancerAccessLog{ - S3BucketName: value, - } + if actual.AccessLog == nil { + actual.AccessLog = &NetworkLoadBalancerAccessLog{} } + actual.AccessLog.S3BucketName = value case "access_logs.s3.prefix": - if actual.AccessLog != nil { - actual.AccessLog.S3BucketPrefix = value - } else { - actual.AccessLog = &NetworkLoadBalancerAccessLog{ - S3BucketPrefix: value, - } + if actual.AccessLog == nil { + actual.AccessLog = &NetworkLoadBalancerAccessLog{} } + actual.AccessLog.S3BucketPrefix = value default: klog.V(2).Infof("unsupported key -- ignoring, %v.\n", key) } @@ -706,6 +697,7 @@ type terraformNetworkLoadBalancer struct { Type string `json:"load_balancer_type" cty:"load_balancer_type"` SubnetMappings []terraformNetworkLoadBalancerSubnetMapping `json:"subnet_mapping" cty:"subnet_mapping"` CrossZoneLoadBalancing bool `json:"enable_cross_zone_load_balancing" cty:"enable_cross_zone_load_balancing"` + AccessLog *terraformNetworkLoadBalancerAccessLog `json:"access_logs,omitempty" cty:"access_logs"` Tags map[string]string `json:"tags" cty:"tags"` } @@ -747,6 +739,14 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e }) } + if e.AccessLog != nil && fi.BoolValue(e.AccessLog.Enabled) { + nlbTF.AccessLog = &terraformNetworkLoadBalancerAccessLog{ + Enabled: e.AccessLog.Enabled, + S3BucketName: e.AccessLog.S3BucketName, + S3BucketPrefix: e.AccessLog.S3BucketPrefix, + } + } + err := t.RenderResource("aws_lb", *e.Name, nlbTF) if err != nil { return err @@ -788,6 +788,7 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e return err } } + return nil } @@ -800,11 +801,12 @@ func (e *NetworkLoadBalancer) TerraformLink(params ...string) *terraformWriter.L } type cloudformationNetworkLoadBalancer struct { - Name string `json:"Name"` - Scheme string `json:"Scheme"` - SubnetMappings []*cloudformationSubnetMapping `json:"SubnetMappings"` - Type string `json:"Type"` - Tags []cloudformationTag `json:"Tags"` + Name string `json:"Name"` + Scheme string `json:"Scheme"` + SubnetMappings []*cloudformationSubnetMapping `json:"SubnetMappings"` + Type string `json:"Type"` + Tags []cloudformationTag `json:"Tags"` + LoadBalancerAttributes []cloudformationLoadBalancerAttribute `json:"LoadBalancerAttributes,omitempty"` } type cloudformationSubnetMapping struct { @@ -813,6 +815,11 @@ type cloudformationSubnetMapping struct { PrivateIPv4Address *string `json:"PrivateIPv4Address,omitempty"` } +type cloudformationLoadBalancerAttribute struct { + Key *string `json:"Key"` + Value *string `json:"Value,omitempty"` +} + type cloudformationNetworkLoadBalancerListener struct { Certificates []cloudformationNetworkLoadBalancerListenerCertificate `json:"Certificates,omitempty"` DefaultActions []cloudformationNetworkLoadBalancerListenerAction `json:"DefaultActions"` @@ -849,6 +856,25 @@ func (_ *NetworkLoadBalancer) RenderCloudformation(t *cloudformation.Cloudformat } else { nlbCF.Scheme = elbv2.LoadBalancerSchemeEnumInternetFacing } + + if e.AccessLog != nil && *e.AccessLog.Enabled { + var attributes []cloudformationLoadBalancerAttribute + + attributes = append(attributes, cloudformationLoadBalancerAttribute{ + Key: aws.String("access_logs.s3.enabled"), + Value: aws.String(strconv.FormatBool(aws.BoolValue(e.AccessLog.Enabled))), + }) + attributes = append(attributes, cloudformationLoadBalancerAttribute{ + Key: aws.String("access_logs.s3.bucket"), + Value: e.AccessLog.S3BucketName, + }) + attributes = append(attributes, cloudformationLoadBalancerAttribute{ + Key: aws.String("access_logs.s3.prefix"), + Value: e.AccessLog.S3BucketPrefix, + }) + nlbCF.LoadBalancerAttributes = attributes + } + err := t.RenderResource("AWS::ElasticLoadBalancingV2::LoadBalancer", *e.Name, nlbCF) if err != nil { return err diff --git a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go index 3c307967f2..460f25f930 100644 --- a/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go +++ b/upup/pkg/fi/cloudup/awstasks/networkloadbalancer_attributes.go @@ -37,6 +37,12 @@ func (_ *NetworkLoadBalancerAccessLog) GetDependencies(tasks map[string]fi.Task) return nil } +type terraformNetworkLoadBalancerAccessLog struct { + Enabled *bool `json:"enabled,omitempty" cty:"enabled"` + S3BucketName *string `json:"bucket,omitempty" cty:"bucket"` + S3BucketPrefix *string `json:"bucket_prefix,omitempty" cty:"prefix"` +} + func findNetworkLoadBalancerAttributes(cloud awsup.AWSCloud, LoadBalancerArn string) ([]*elbv2.LoadBalancerAttribute, error) { request := &elbv2.DescribeLoadBalancerAttributesInput{