From 9d1952597e2b5f53c8ab06fd1783d5cf50192350 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Mon, 22 Apr 2024 10:24:01 +0100 Subject: [PATCH 01/10] Fetch vpcid from runtime using vpc flags --- docs/deploy/configurations.md | 92 ++++++++++++++++++----------------- docs/deploy/installation.md | 2 +- pkg/aws/cloud.go | 47 +++++++++++++++--- pkg/aws/cloud_config.go | 11 +++++ pkg/aws/services/ec2.go | 23 +++++++-- 5 files changed, 119 insertions(+), 56 deletions(-) diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md index 3974f127a..6314a87e3 100644 --- a/docs/deploy/configurations.md +++ b/docs/deploy/configurations.md @@ -64,51 +64,53 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne !!!warning "" The --cluster-name flag is mandatory and the value must match the name of the kubernetes cluster. If you specify an incorrect name, the subnet auto-discovery will not work. -|Flag | Type | Default | Description | -|---------------------------------------|---------------------------------|-----------------|-------------| -|aws-api-endpoints | AWS API Endpoints Config | | AWS API endpoints mapping, format: serviceID1=URL1,serviceID2=URL2 | -|aws-api-throttle | AWS Throttle Config | [default value](#default-throttle-config ) | throttle settings for AWS APIs, format: serviceID1:operationRegex1=rate:burst,serviceID2:operationRegex2=rate:burst | -|aws-max-retries | int | 10 | Maximum retries for AWS APIs | -|aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster | -|aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster | -|allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) | -|backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG| -|cluster-name | string | | Kubernetes cluster name| -|default-ssl-policy | string | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation | -|default-tags | stringMap | | AWS Tags that will be applied to all AWS resources managed by this controller. Specified Tags takes highest priority | -|default-target-type | string | instance | Default target type for Ingresses and Services - ip, instance | -|[disable-ingress-class-annotation](#disable-ingress-class-annotation) | boolean | false | Disable new usage of the `kubernetes.io/ingress.class` annotation | -|[disable-ingress-group-name-annotation](#disable-ingress-group-name-annotation) | boolean | false | Disallow new use of the `alb.ingress.kubernetes.io/group.name` annotation | -|disable-restricted-sg-rules | boolean | false | Disable the usage of restricted security group rules | -|enable-backend-security-group | boolean | true | Enable sharing of security groups for backend traffic | -|enable-endpoint-slices | boolean | false | Use EndpointSlices instead of Endpoints for pod endpoint and TargetGroupBinding resolution for load balancers with IP targets. | -|enable-leader-election | boolean | true | Enable leader election for the load balancer controller manager. Enabling this will ensure there is only one active controller manager | -|enable-pod-readiness-gate-inject | boolean | true | If enabled, targetHealth readiness gate will get injected to the pod spec for the matching endpoint pods | -|enable-shield | boolean | true | Enable Shield addon for ALB | -|[enable-waf](#waf-addons) | boolean | true | Enable WAF addon for ALB | -|[enable-wafv2](#waf-addons) | boolean | true | Enable WAF V2 addon for ALB | -|external-managed-tags | stringList | | AWS Tag keys that will be managed externally. Specified Tags are ignored during reconciliation | -|[feature-gates](#feature-gates) | stringMap | | A set of key=value pairs to enable or disable features | -|health-probe-bind-addr | string | :61779 | The address the health probes binds to | -|ingress-class | string | alb | Name of the ingress class this controller satisfies | -|ingress-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for ingress | -|kubeconfig | string | in-cluster config | Path to the kubeconfig file containing authorization and API server information | -|leader-election-id | string | aws-load-balancer-controller-leader | Name of the leader election ID to use for this controller | -|leader-election-namespace | string | | Name of the leader election ID to use for this controller | -|load-balancer-class | string | service.k8s.aws/nlb| Name of the load balancer class specified in service `spec.loadBalancerClass` reconciled by this controller | -|log-level | string | info | Set the controller log level - info, debug | -|metrics-bind-addr | string | :8080 | The address the metric endpoint binds to | -|service-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for service | -|[sync-period](#sync-period) | duration | 10h0m0s | Period at which the controller forces the repopulation of its local object stores| -|targetgroupbinding-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for targetGroupBinding | -|targetgroupbinding-max-exponential-backoff-delay | duration | 16m40s | Maximum duration of exponential backoff for targetGroupBinding reconcile failures | -|tolerate-non-existent-backend-service | boolean | true | Whether to allow rules which refer to backend services that do not exist (When enabled, it will return 503 error if backend service not exist) | -|tolerate-non-existent-backend-action | boolean | true | Whether to allow rules which refer to backend actions that do not exist (When enabled, it will return 503 error if backend action not exist) | -|watch-namespace | string | | Namespace the controller watches for updates to Kubernetes objects, If empty, all namespaces are watched. | -|webhook-bind-port | int | 9443 | The TCP port the Webhook server binds to | -|webhook-cert-dir | string | /tmp/k8s-webhook-server/serving-certs | The directory that contains the server key and certificate | -|webhook-cert-file | string | tls.crt | The server certificate name | -|webhook-key-file | string | tls.key | The server key name | +| Flag | Type | Default | Description | +|---------------------------------------------------------------------------------|---------------------------------|--------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------| +| aws-api-endpoints | AWS API Endpoints Config | | AWS API endpoints mapping, format: serviceID1=URL1,serviceID2=URL2 | +| aws-api-throttle | AWS Throttle Config | [default value](#default-throttle-config ) | throttle settings for AWS APIs, format: serviceID1:operationRegex1=rate:burst,serviceID2:operationRegex2=rate:burst | +| aws-max-retries | int | 10 | Maximum retries for AWS APIs | +| aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster | +| aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster | +| aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC +| aws-vpc-name-tag-key | string | Name | Optional tag key used with aws-vpc-tags add only if VPC name tag key is not the default value "Name" +| allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) | +| backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG | +| cluster-name | string | | Kubernetes cluster name | +| default-ssl-policy | string | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation | +| default-tags | stringMap | | AWS Tags that will be applied to all AWS resources managed by this controller. Specified Tags takes highest priority | +| default-target-type | string | instance | Default target type for Ingresses and Services - ip, instance | +| [disable-ingress-class-annotation](#disable-ingress-class-annotation) | boolean | false | Disable new usage of the `kubernetes.io/ingress.class` annotation | +| [disable-ingress-group-name-annotation](#disable-ingress-group-name-annotation) | boolean | false | Disallow new use of the `alb.ingress.kubernetes.io/group.name` annotation | +| disable-restricted-sg-rules | boolean | false | Disable the usage of restricted security group rules | +| enable-backend-security-group | boolean | true | Enable sharing of security groups for backend traffic | +| enable-endpoint-slices | boolean | false | Use EndpointSlices instead of Endpoints for pod endpoint and TargetGroupBinding resolution for load balancers with IP targets. | +| enable-leader-election | boolean | true | Enable leader election for the load balancer controller manager. Enabling this will ensure there is only one active controller manager | +| enable-pod-readiness-gate-inject | boolean | true | If enabled, targetHealth readiness gate will get injected to the pod spec for the matching endpoint pods | +| enable-shield | boolean | true | Enable Shield addon for ALB | +| [enable-waf](#waf-addons) | boolean | true | Enable WAF addon for ALB | +| [enable-wafv2](#waf-addons) | boolean | true | Enable WAF V2 addon for ALB | +| external-managed-tags | stringList | | AWS Tag keys that will be managed externally. Specified Tags are ignored during reconciliation | +| [feature-gates](#feature-gates) | stringMap | | A set of key=value pairs to enable or disable features | +| health-probe-bind-addr | string | :61779 | The address the health probes binds to | +| ingress-class | string | alb | Name of the ingress class this controller satisfies | +| ingress-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for ingress | +| kubeconfig | string | in-cluster config | Path to the kubeconfig file containing authorization and API server information | +| leader-election-id | string | aws-load-balancer-controller-leader | Name of the leader election ID to use for this controller | +| leader-election-namespace | string | | Name of the leader election ID to use for this controller | +| load-balancer-class | string | service.k8s.aws/nlb | Name of the load balancer class specified in service `spec.loadBalancerClass` reconciled by this controller | +| log-level | string | info | Set the controller log level - info, debug | +| metrics-bind-addr | string | :8080 | The address the metric endpoint binds to | +| service-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for service | +| [sync-period](#sync-period) | duration | 10h0m0s | Period at which the controller forces the repopulation of its local object stores | +| targetgroupbinding-max-concurrent-reconciles | int | 3 | Maximum number of concurrently running reconcile loops for targetGroupBinding | +| targetgroupbinding-max-exponential-backoff-delay | duration | 16m40s | Maximum duration of exponential backoff for targetGroupBinding reconcile failures | +| tolerate-non-existent-backend-service | boolean | true | Whether to allow rules which refer to backend services that do not exist (When enabled, it will return 503 error if backend service not exist) | +| tolerate-non-existent-backend-action | boolean | true | Whether to allow rules which refer to backend actions that do not exist (When enabled, it will return 503 error if backend action not exist) | +| watch-namespace | string | | Namespace the controller watches for updates to Kubernetes objects, If empty, all namespaces are watched. | +| webhook-bind-port | int | 9443 | The TCP port the Webhook server binds to | +| webhook-cert-dir | string | /tmp/k8s-webhook-server/serving-certs | The directory that contains the server key and certificate | +| webhook-cert-file | string | tls.crt | The server certificate name | +| webhook-key-file | string | tls.key | The server key name | ### disable-ingress-class-annotation diff --git a/docs/deploy/installation.md b/docs/deploy/installation.md index 5e42d6266..977148d46 100644 --- a/docs/deploy/installation.md +++ b/docs/deploy/installation.md @@ -37,7 +37,7 @@ You can set the IMDSv2 as follows: aws ec2 modify-instance-metadata-options --http-put-response-hop-limit 2 --http-tokens required --region --instance-id ``` -Instead of depending on IMDSv2, you can specify the AWS Region and the VPC via the controller flags `--aws-region` and `--aws-vpc-id`. +Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-name-tag-key` if you have a different key for the tag other than "Name". ## Configure IAM diff --git a/pkg/aws/cloud.go b/pkg/aws/cloud.go index 403813fce..84bd4ca37 100644 --- a/pkg/aws/cloud.go +++ b/pkg/aws/cloud.go @@ -1,6 +1,7 @@ package aws import ( + "context" "fmt" "net" "os" @@ -112,13 +113,11 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, ec2Service := services.NewEC2(sess) - if len(cfg.VpcID) == 0 { - vpcID, err := inferVPCID(metadata, ec2Service) - if err != nil { - return nil, errors.Wrap(err, "failed to introspect vpcID from EC2Metadata or Node name, specify --aws-vpc-id instead if EC2Metadata is unavailable") - } - cfg.VpcID = vpcID + vpcID, err := getVpcID(cfg, ec2Service, metadata) + if err != nil { + return nil, errors.Wrap(err, "failed to get VPC ID") } + cfg.VpcID = vpcID return &defaultCloud{ cfg: cfg, @@ -132,6 +131,18 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, }, nil } +func getVpcID(cfg CloudConfig, ec2Service services.EC2, metadata services.EC2Metadata) (string, error) { + if cfg.VpcID != "" { + return cfg.VpcID, nil + } + + if cfg.VpcTags != nil { + return inferVPCIDFromTags(ec2Service, cfg.VpcTags[cfg.VpcNameTagKey]) + } + + return inferVPCID(metadata, ec2Service) +} + func inferVPCID(metadata services.EC2Metadata, ec2Service services.EC2) (string, error) { var errList []error vpcId, err := metadata.VpcID() @@ -168,6 +179,30 @@ func inferVPCID(metadata services.EC2Metadata, ec2Service services.EC2) (string, return "", amerrors.NewAggregate(errList) } +func inferVPCIDFromTags(ec2Service services.EC2, vpcNameTagKey string) (string, error) { + vpcs, err := ec2Service.DescribeVPCsAsList(context.Background(), &ec2.DescribeVpcsInput{ + Filters: []*ec2.Filter{ + { + Name: &vpcNameTagKey, + //Values: []*string{ + // aws.String("owned"), + //}, + }, + }, + }) + if err != nil { + return "", fmt.Errorf("failed to fetch VPC ID with tag: %w", err) + } + if len(vpcs) == 0 { + return "", fmt.Errorf("no VPC exists with tag: %w", err) + } + if len(vpcs) > 1 { + return "", fmt.Errorf("multiple VPCs exists with tag: %w", err) + } + + return *vpcs[0].VpcId, nil +} + var _ Cloud = &defaultCloud{} type defaultCloud struct { diff --git a/pkg/aws/cloud_config.go b/pkg/aws/cloud_config.go index 784c5602d..30be1b5be 100644 --- a/pkg/aws/cloud_config.go +++ b/pkg/aws/cloud_config.go @@ -12,9 +12,12 @@ const ( flagAWSAPIEndpoints = "aws-api-endpoints" flagAWSAPIThrottle = "aws-api-throttle" flagAWSVpcID = "aws-vpc-id" + flagAWSVpcTags = "aws-vpc-tags" flagAWSVpcCacheTTL = "aws-vpc-cache-ttl" flagAWSMaxRetries = "aws-max-retries" + flagAWSVpcNameTagKey = "aws-vpc-name-tag-key" defaultVpcID = "" + defaultVpcNameTagKey = "Name" defaultRegion = "" defaultAPIMaxRetries = 10 ) @@ -29,6 +32,12 @@ type CloudConfig struct { // VpcID for the LoadBalancer resources. VpcID string + // VPC tags List + VpcTags map[string]string + + // VPC Name Tag Key, default "Name" + VpcNameTagKey string + // VPC cache TTL in minutes VpcCacheTTL time.Duration @@ -43,6 +52,8 @@ func (cfg *CloudConfig) BindFlags(fs *pflag.FlagSet) { fs.StringVar(&cfg.Region, flagAWSRegion, defaultRegion, "AWS Region for the kubernetes cluster") fs.Var(cfg.ThrottleConfig, flagAWSAPIThrottle, "throttle settings for AWS APIs, format: serviceID1:operationRegex1=rate:burst,serviceID2:operationRegex2=rate:burst") fs.StringVar(&cfg.VpcID, flagAWSVpcID, defaultVpcID, "AWS VpcID for the LoadBalancer resources") + fs.StringToStringVar(&cfg.VpcTags, flagAWSVpcTags, nil, "AWS VPC tags List,format: tagkey1=tagvalue1,tagkey2=tagvalue2") + fs.StringVar(&cfg.VpcNameTagKey, flagAWSVpcNameTagKey, defaultVpcNameTagKey, "AWS tag key for identifying the VPC") fs.IntVar(&cfg.MaxRetries, flagAWSMaxRetries, defaultAPIMaxRetries, "Maximum retries for AWS APIs") fs.StringToStringVar(&cfg.AWSEndpoints, flagAWSAPIEndpoints, nil, "Custom AWS endpoint configuration, format: serviceID1=URL1,serviceID2=URL2") } diff --git a/pkg/aws/services/ec2.go b/pkg/aws/services/ec2.go index 43c79a2c2..80230f96f 100644 --- a/pkg/aws/services/ec2.go +++ b/pkg/aws/services/ec2.go @@ -2,6 +2,7 @@ package services import ( "context" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" @@ -10,17 +11,20 @@ import ( type EC2 interface { ec2iface.EC2API - // wrapper to DescribeInstancesPagesWithContext API, which aggregates paged results into list. + // DescribeInstancesAsList wraps the DescribeInstancesPagesWithContext API, which aggregates paged results into list. DescribeInstancesAsList(ctx context.Context, input *ec2.DescribeInstancesInput) ([]*ec2.Instance, error) - // wrapper to DescribeNetworkInterfacesPagesWithContext API, which aggregates paged results into list. + // DescribeNetworkInterfacesAsList wraps the DescribeNetworkInterfacesPagesWithContext API, which aggregates paged results into list. DescribeNetworkInterfacesAsList(ctx context.Context, input *ec2.DescribeNetworkInterfacesInput) ([]*ec2.NetworkInterface, error) - // wrapper to DescribeSecurityGroupsPagesWithContext API, which aggregates paged results into list. + // DescribeSecurityGroupsAsList wraps the DescribeSecurityGroupsPagesWithContext API, which aggregates paged results into list. DescribeSecurityGroupsAsList(ctx context.Context, input *ec2.DescribeSecurityGroupsInput) ([]*ec2.SecurityGroup, error) - // wrapper to DescribeSubnetsPagesWithContext API, which aggregates paged results into list. + // DescribeSubnetsAsList wraps the DescribeSubnetsPagesWithContext API, which aggregates paged results into list. DescribeSubnetsAsList(ctx context.Context, input *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) + + // DescribeVPCsAsList wraps the DescribeVpcsPagesWithContext API, which aggregates paged results into list. + DescribeVPCsAsList(ctx context.Context, input *ec2.DescribeVpcsInput) ([]*ec2.Vpc, error) } // NewEC2 constructs new EC2 implementation. @@ -79,3 +83,14 @@ func (c *defaultEC2) DescribeSubnetsAsList(ctx context.Context, input *ec2.Descr } return result, nil } + +func (c *defaultEC2) DescribeVPCsAsList(ctx context.Context, input *ec2.DescribeVpcsInput) ([]*ec2.Vpc, error) { + var result []*ec2.Vpc + if err := c.DescribeVpcsPagesWithContext(ctx, input, func(output *ec2.DescribeVpcsOutput, _ bool) bool { + result = append(result, output.Vpcs...) + return true + }); err != nil { + return nil, err + } + return result, nil +} From 69a5d42c2b3acc70788f342216f178d1b4e4427e Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Mon, 22 Apr 2024 12:07:49 +0100 Subject: [PATCH 02/10] added tests related to changes using mockgen --- pkg/aws/services/ec2_mocks.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/aws/services/ec2_mocks.go b/pkg/aws/services/ec2_mocks.go index 0e2419f51..e3ffd34e3 100644 --- a/pkg/aws/services/ec2_mocks.go +++ b/pkg/aws/services/ec2_mocks.go @@ -20597,6 +20597,21 @@ func (mr *MockEC2MockRecorder) DescribeTrunkInterfaceAssociationsWithContext(arg return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeTrunkInterfaceAssociationsWithContext", reflect.TypeOf((*MockEC2)(nil).DescribeTrunkInterfaceAssociationsWithContext), varargs...) } +// DescribeVPCsAsList mocks base method. +func (m *MockEC2) DescribeVPCsAsList(arg0 context.Context, arg1 *ec2.DescribeVpcsInput) ([]*ec2.Vpc, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DescribeVPCsAsList", arg0, arg1) + ret0, _ := ret[0].([]*ec2.Vpc) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DescribeVPCsAsList indicates an expected call of DescribeVPCsAsList. +func (mr *MockEC2MockRecorder) DescribeVPCsAsList(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeVPCsAsList", reflect.TypeOf((*MockEC2)(nil).DescribeVPCsAsList), arg0, arg1) +} + // DescribeVerifiedAccessEndpoints mocks base method. func (m *MockEC2) DescribeVerifiedAccessEndpoints(arg0 *ec2.DescribeVerifiedAccessEndpointsInput) (*ec2.DescribeVerifiedAccessEndpointsOutput, error) { m.ctrl.T.Helper() From 8ae3e3c10ffc5a9c6a22d105ee9bcfdf96838e20 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Thu, 9 May 2024 21:13:50 +0100 Subject: [PATCH 03/10] fixed the filter error --- pkg/aws/cloud.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/aws/cloud.go b/pkg/aws/cloud.go index 84bd4ca37..d3201b8c1 100644 --- a/pkg/aws/cloud.go +++ b/pkg/aws/cloud.go @@ -118,7 +118,6 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, return nil, errors.Wrap(err, "failed to get VPC ID") } cfg.VpcID = vpcID - return &defaultCloud{ cfg: cfg, ec2: ec2Service, @@ -137,7 +136,7 @@ func getVpcID(cfg CloudConfig, ec2Service services.EC2, metadata services.EC2Met } if cfg.VpcTags != nil { - return inferVPCIDFromTags(ec2Service, cfg.VpcTags[cfg.VpcNameTagKey]) + return inferVPCIDFromTags(ec2Service, cfg.VpcNameTagKey, cfg.VpcTags[cfg.VpcNameTagKey]) } return inferVPCID(metadata, ec2Service) @@ -179,14 +178,12 @@ func inferVPCID(metadata services.EC2Metadata, ec2Service services.EC2) (string, return "", amerrors.NewAggregate(errList) } -func inferVPCIDFromTags(ec2Service services.EC2, vpcNameTagKey string) (string, error) { +func inferVPCIDFromTags(ec2Service services.EC2, VpcNameTagKey string, VpcNameTagValue string) (string, error) { vpcs, err := ec2Service.DescribeVPCsAsList(context.Background(), &ec2.DescribeVpcsInput{ Filters: []*ec2.Filter{ { - Name: &vpcNameTagKey, - //Values: []*string{ - // aws.String("owned"), - //}, + Name: aws.String("tag:" + VpcNameTagKey), + Values: []*string{aws.String(VpcNameTagValue)}, }, }, }) From 3a0ebcef99c41ed68b20f34d6e570d239d1d86c2 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 00:07:09 +0100 Subject: [PATCH 04/10] change vpc tag key name to more generic --- docs/deploy/configurations.md | 2 +- docs/deploy/installation.md | 2 +- pkg/aws/cloud_config.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md index 6314a87e3..1eb9db263 100644 --- a/docs/deploy/configurations.md +++ b/docs/deploy/configurations.md @@ -72,7 +72,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne | aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster | | aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster | | aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC -| aws-vpc-name-tag-key | string | Name | Optional tag key used with aws-vpc-tags add only if VPC name tag key is not the default value "Name" +| aws-vpc-tag-key | string | Name | Optional tag key used with aws-vpc-tags add only if VPC name tag key is not the default value "Name" | allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) | | backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG | | cluster-name | string | | Kubernetes cluster name | diff --git a/docs/deploy/installation.md b/docs/deploy/installation.md index 977148d46..da89e7de1 100644 --- a/docs/deploy/installation.md +++ b/docs/deploy/installation.md @@ -37,7 +37,7 @@ You can set the IMDSv2 as follows: aws ec2 modify-instance-metadata-options --http-put-response-hop-limit 2 --http-tokens required --region --instance-id ``` -Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-name-tag-key` if you have a different key for the tag other than "Name". +Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". ## Configure IAM diff --git a/pkg/aws/cloud_config.go b/pkg/aws/cloud_config.go index 30be1b5be..079321565 100644 --- a/pkg/aws/cloud_config.go +++ b/pkg/aws/cloud_config.go @@ -15,7 +15,7 @@ const ( flagAWSVpcTags = "aws-vpc-tags" flagAWSVpcCacheTTL = "aws-vpc-cache-ttl" flagAWSMaxRetries = "aws-max-retries" - flagAWSVpcNameTagKey = "aws-vpc-name-tag-key" + flagAWSVpcNameTagKey = "aws-vpc-tag-key" defaultVpcID = "" defaultVpcNameTagKey = "Name" defaultRegion = "" From e20e081f95c06b7cf7f393a0b7df5afc4f2375c9 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 00:44:50 +0100 Subject: [PATCH 05/10] add logs about the vpc values fetched by the controller --- docs/deploy/installation.md | 2 +- pkg/aws/cloud.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/deploy/installation.md b/docs/deploy/installation.md index da89e7de1..bdb801101 100644 --- a/docs/deploy/installation.md +++ b/docs/deploy/installation.md @@ -37,7 +37,7 @@ You can set the IMDSv2 as follows: aws ec2 modify-instance-metadata-options --http-put-response-hop-limit 2 --http-tokens required --region --instance-id ``` -Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". +Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". Note that if you specify flags `--aws-vpc-id` and `--aws-vpc-tags`, then value given `--aws-vpc-id` will be taken by controller. ## Configure IAM diff --git a/pkg/aws/cloud.go b/pkg/aws/cloud.go index d3201b8c1..10c4eb298 100644 --- a/pkg/aws/cloud.go +++ b/pkg/aws/cloud.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" amerrors "k8s.io/apimachinery/pkg/util/errors" @@ -131,7 +132,10 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, } func getVpcID(cfg CloudConfig, ec2Service services.EC2, metadata services.EC2Metadata) (string, error) { + + logger := logr.Logger{} if cfg.VpcID != "" { + logger.V(1).Info("vpcid is specified using flag --aws-vpc-id, controller will use the value %s", cfg.VpcID) return cfg.VpcID, nil } From 443d102b64df6a00d9ff48286d093447ebb37127 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 00:45:53 +0100 Subject: [PATCH 06/10] improve the doc changes --- docs/deploy/installation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/deploy/installation.md b/docs/deploy/installation.md index bdb801101..663970e5d 100644 --- a/docs/deploy/installation.md +++ b/docs/deploy/installation.md @@ -37,7 +37,7 @@ You can set the IMDSv2 as follows: aws ec2 modify-instance-metadata-options --http-put-response-hop-limit 2 --http-tokens required --region --instance-id ``` -Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". Note that if you specify flags `--aws-vpc-id` and `--aws-vpc-tags`, then value given `--aws-vpc-id` will be taken by controller. +Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". Note that if you specify flags `--aws-vpc-id` and `--aws-vpc-tags`, then value given to `--aws-vpc-id` will be taken by controller. ## Configure IAM From 22b2e6b9708057bad6c4ab287c86f6b066df143e Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 01:28:22 +0100 Subject: [PATCH 07/10] improve the doc changes --- docs/deploy/configurations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md index 1eb9db263..5c1773d7c 100644 --- a/docs/deploy/configurations.md +++ b/docs/deploy/configurations.md @@ -71,7 +71,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne | aws-max-retries | int | 10 | Maximum retries for AWS APIs | | aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster | | aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster | -| aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC +| aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC, Note that if both flags --aws-vpc-id and --aws-vpc-tags are specified, the controller uses the value in --aws-vpc-id to fetch the VPC info and ignores the other flag. | aws-vpc-tag-key | string | Name | Optional tag key used with aws-vpc-tags add only if VPC name tag key is not the default value "Name" | allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) | | backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG | From 7b3f80d98ad99f71e388d92ef47e77317b5db4cc Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 01:33:08 +0100 Subject: [PATCH 08/10] improve the doc changes --- docs/deploy/configurations.md | 2 +- docs/deploy/installation.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md index 5c1773d7c..a891e552b 100644 --- a/docs/deploy/configurations.md +++ b/docs/deploy/configurations.md @@ -71,7 +71,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne | aws-max-retries | int | 10 | Maximum retries for AWS APIs | | aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster | | aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster | -| aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC, Note that if both flags --aws-vpc-id and --aws-vpc-tags are specified, the controller uses the value in --aws-vpc-id to fetch the VPC info and ignores the other flag. +| aws-vpc-tags | stringMap | | Tags for the Kubernetes cluster VPC, When both flags `--aws-vpc-id` and `--aws-vpc-tags` are specified, the controller prioritizes `--aws-vpc-id` and ignores the other flag. | aws-vpc-tag-key | string | Name | Optional tag key used with aws-vpc-tags add only if VPC name tag key is not the default value "Name" | allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) | | backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG | diff --git a/docs/deploy/installation.md b/docs/deploy/installation.md index 663970e5d..146417d23 100644 --- a/docs/deploy/installation.md +++ b/docs/deploy/installation.md @@ -37,7 +37,7 @@ You can set the IMDSv2 as follows: aws ec2 modify-instance-metadata-options --http-put-response-hop-limit 2 --http-tokens required --region --instance-id ``` -Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". Note that if you specify flags `--aws-vpc-id` and `--aws-vpc-tags`, then value given to `--aws-vpc-id` will be taken by controller. +Instead of depending on IMDSv2, you can specify the AWS Region via the controller flag `--aws-region`, and the AWS VPC via controller flag `--aws-vpc-id` or by specifying vpc tags via the flag `--aws-vpc-tags` and an optional flag `--aws-vpc-tag-key` if you have a different key for the tag other than "Name". When both flags `--aws-vpc-id` and `--aws-vpc-tags` are specified, the controller prioritizes `--aws-vpc-id`and ignores the other flag. ## Configure IAM From 663315e134c028f6b02b08876a133f314d166970 Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 22:43:20 +0100 Subject: [PATCH 09/10] fixed the logger issue --- main.go | 3 ++- pkg/aws/cloud.go | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/main.go b/main.go index 88cc1755a..5b1682b77 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ package main import ( "os" + elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2" "github.com/go-logr/logr" @@ -76,7 +77,7 @@ func main() { } ctrl.SetLogger(getLoggerWithLogLevel(controllerCFG.LogLevel)) - cloud, err := aws.NewCloud(controllerCFG.AWSConfig, metrics.Registry) + cloud, err := aws.NewCloud(controllerCFG.AWSConfig, metrics.Registry, ctrl.Log) if err != nil { setupLog.Error(err, "unable to initialize AWS cloud") os.Exit(1) diff --git a/pkg/aws/cloud.go b/pkg/aws/cloud.go index 10c4eb298..faba5cec4 100644 --- a/pkg/aws/cloud.go +++ b/pkg/aws/cloud.go @@ -51,7 +51,7 @@ type Cloud interface { } // NewCloud constructs new Cloud implementation. -func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, error) { +func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer, logger logr.Logger) (Cloud, error) { hasIPv4 := true addrs, err := net.InterfaceAddrs() if err == nil { @@ -114,7 +114,7 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, ec2Service := services.NewEC2(sess) - vpcID, err := getVpcID(cfg, ec2Service, metadata) + vpcID, err := getVpcID(cfg, ec2Service, metadata, logger) if err != nil { return nil, errors.Wrap(err, "failed to get VPC ID") } @@ -131,11 +131,10 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, }, nil } -func getVpcID(cfg CloudConfig, ec2Service services.EC2, metadata services.EC2Metadata) (string, error) { +func getVpcID(cfg CloudConfig, ec2Service services.EC2, metadata services.EC2Metadata, logger logr.Logger) (string, error) { - logger := logr.Logger{} if cfg.VpcID != "" { - logger.V(1).Info("vpcid is specified using flag --aws-vpc-id, controller will use the value %s", cfg.VpcID) + logger.V(1).Info("vpcid is specified using flag --aws-vpc-id, controller will use the value", "vpc: ", cfg.VpcID) return cfg.VpcID, nil } From 8a0147826cc45be2f824dcdbf630ff95ff85408f Mon Sep 17 00:00:00 2001 From: jeswinkoshyninan Date: Tue, 14 May 2024 22:52:22 +0100 Subject: [PATCH 10/10] update unit test case --- test/framework/framework.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/framework/framework.go b/test/framework/framework.go index cb042227f..4402f16d1 100644 --- a/test/framework/framework.go +++ b/test/framework/framework.go @@ -55,18 +55,18 @@ func InitFramework() (*Framework, error) { return nil, err } + logger, loggerReporter := utils.NewGinkgoLogger() + cloud, err := aws.NewCloud(aws.CloudConfig{ Region: globalOptions.AWSRegion, VpcID: globalOptions.AWSVPCID, MaxRetries: 3, ThrottleConfig: throttle.NewDefaultServiceOperationsThrottleConfig(), - }, nil) + }, nil, logger) if err != nil { return nil, err } - logger, loggerReporter := utils.NewGinkgoLogger() - f := &Framework{ Options: globalOptions, RestCfg: restCfg,