Fetch Region and CLUSTER_ID information from cni-metrics-helper env #1715

cgchinmay · 2021-10-28T22:52:38Z

What type of PR is this?
feature

Which issue does this PR fix:
Allows Customers to use cni-metrics-helper by completely blocking access to IMDS

What does this PR do / Why do we need it:
Removes hard dependency of cni-metrics-helper on IMDS

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
#1287

Testing done on this change:
Automation added to e2e:
Added Unit test
Manually tested on sample cluster

Tested by disabling pod-imds

"MetadataOptions": {
                        "State": "applied",
                        "HttpTokens": "required",
                        "HttpPutResponseHopLimit": 1,
                        "HttpEndpoint": "enabled"
                    },

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No

Does this change require updates to the CNI daemonset config files to work?:

Yes, if IMDS access needs to be blocked

Does this PR introduce any user-facing change?:
Yes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

cmd/cni-metrics-helper/metrics/metrics.go

pkg/publisher/publisher.go

config/master/cni-metrics-helper.yaml

jayanthvn

Can you also add a section here - https://github.com/aws/amazon-vpc-cni-k8s/tree/master/cmd/cni-metrics-helper. We can give steps on how to enable IRSA for cni-metrics helper and include this -

Case 1: Cx not using IRSA, we need to get region and clusterID using IMDS (existing approach)
 	// Case 2: Cx using IRSA but not specified clusterID, we can still get this info if IMDS is not blocked
 	// Case 3: Cx blocked IMDS access and not using IRSA (which means region == "") OR
 	// not specified clusterID then its a Cx error

jayanthvn · 2021-11-02T14:49:20Z

Also as part of this, lets update https://docs.aws.amazon.com/eks/latest/userguide/cni-metrics-helper.html for IRSA support.

config/master/manifests.jsonnet

pkg/publisher/publisher.go

cgchinmay · 2021-11-09T16:11:29Z

Can you also add a section here - https://github.com/aws/amazon-vpc-cni-k8s/tree/master/cmd/cni-metrics-helper. We can give steps on how to enable IRSA for cni-metrics helper and include this -
Case 1: Cx not using IRSA, we need to get region and clusterID using IMDS (existing approach)
 	// Case 2: Cx using IRSA but not specified clusterID, we can still get this info if IMDS is not blocked
 	// Case 3: Cx blocked IMDS access and not using IRSA (which means region == "") OR
 	// not specified clusterID then its a Cx error

Updated Readme

Fix compilation errors (aws#1751) add support for running canary script in different regions (aws#1752) Regenerate pod eni values for new instance types (aws#1754) * Regenerate pod eni values for new instance types Co-authored-by: Senthil Kumaran <senthilx@amazon.com> Closed issue message (aws#1761) * closed issue message * update message fix typo in upload script (aws#1763) Update calico file path Use an unique s3 bucket name (aws#1760) Update region Workflow to build arm and x86 images (aws#1764) DataStore.GetStats() refactoring to simplify adding new fields (aws#1704) * DataStore.GetStats() refactoring to simplify adding new fields * cleanup * cleanup * cleanup * goimports * rename test to TestGetStatsV4 * address comments * fix typo * update * update "IP pool is too low" logging * GetStats() -> GetIpStats() * GetStats() -> GetIpStats() in tests and comments * update test * cleanup test * add logPoolStats comment Fix KOPS_STATE_STORE (aws#1770) Automation script for running IT (aws#1759) Update issue template Update issue template with email address Update issue template Update go.mod for integration folder (aws#1741) * Update go.mod for integration folder - Update go.mod for integration folder * Change integration test to use new K8s test framework * Modify server pod image * Switch to Nginx port 80 for server pod * Switch server port in client test * Remove custom command directive for Nginx pod * Added ping command for host checks README: mention arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy (aws#1768) Co-authored-by: Shreya027 <shrenaik@amazon.com> Add dl1.24xlarge to ENILimits override list (aws#1777) Chart and Manifest updates (aws#1771) * Chart and Manifest updates * Update probe timeout values Change workflow to use git install (aws#1785) - Change workflow to use git install as the go get command was altering go.mod file without updating go.sum file Add HostNetworking Test for PPSG in test agent (aws#1720) * Add HostNetworking Test for PPSG in test agent * Updated PPSG test to validate vlan.eth link

Fix for cni-metrics-helper failing integration test

groodt · 2022-01-10T05:04:53Z

Nice! Any ETA on v1.10.2 being GA?

cgchinmay · 2022-01-10T05:49:39Z

Nice! Any ETA on v1.10.2 being GA?

Will soon have one, but will be around mid February. If you want to use this change then we do have private image release. Change the image tag to v1.10.2-rc1 in this manifest and then apply this manifest

groodt · 2022-01-10T11:21:16Z

Change the image tag to v1.10.2-rc1 and apply this manifest.

I've tried upgrading amazon-vpc-cni-k8s to v1.10.2-rc1 and unfortunately aws-node fails to start.

{"level":"info","ts":"2022-01-10T11:17:44.242Z","caller":"entrypoint.sh","msg":"Validating env variables ..."}
{"level":"info","ts":"2022-01-10T11:17:44.339Z","caller":"entrypoint.sh","msg":"Install CNI binaries.."}
{"level":"info","ts":"2022-01-10T11:17:44.644Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
{"level":"info","ts":"2022-01-10T11:17:44.739Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
{"level":"info","ts":"2022-01-10T11:17:47.041Z","caller":"entrypoint.sh","msg":"Retrying waiting for IPAM-D"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x39 pc=0x55ed00eac7c8]

goroutine 552 [running]:
github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd.(*IPAMContext).StartNodeIPPoolManager(0x0)
	/go/src/github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/ipamd.go:633 +0x28
created by main._main
	/go/src/github.com/aws/amazon-vpc-cni-k8s/cmd/aws-k8s-agent/main.go:64 +0x32c
{"level":"info","ts":"2022-01-10T11:17:49.540Z","caller":"entrypoint.sh","msg":"Retrying waiting for IPAM-D"}

cgchinmay · 2022-01-10T11:24:16Z

Change the image tag to v1.10.2-rc1 and apply this manifest.

I've tried upgrading amazon-vpc-cni-k8s to v1.10.2-rc1 and unfortunately aws-node fails to start.

{"level":"info","ts":"2022-01-10T11:17:44.242Z","caller":"entrypoint.sh","msg":"Validating env variables ..."}
{"level":"info","ts":"2022-01-10T11:17:44.339Z","caller":"entrypoint.sh","msg":"Install CNI binaries.."}
{"level":"info","ts":"2022-01-10T11:17:44.644Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
{"level":"info","ts":"2022-01-10T11:17:44.739Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
{"level":"info","ts":"2022-01-10T11:17:47.041Z","caller":"entrypoint.sh","msg":"Retrying waiting for IPAM-D"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x39 pc=0x55ed00eac7c8]

goroutine 552 [running]:
github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd.(*IPAMContext).StartNodeIPPoolManager(0x0)
	/go/src/github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/ipamd.go:633 +0x28
created by main._main
	/go/src/github.com/aws/amazon-vpc-cni-k8s/cmd/aws-k8s-agent/main.go:64 +0x32c
{"level":"info","ts":"2022-01-10T11:17:49.540Z","caller":"entrypoint.sh","msg":"Retrying waiting for IPAM-D"}

Add these env vars to your aws-node ds

name: ENABLE_IPv4
value: "true"
name: ENABLE_IPv6
value: "false"

Or instead apply the manifest for v1.10.1 first and then change the image tag.

groodt · 2022-01-10T12:30:50Z

Or instead apply the manifest for v1.10.1 first and then change the image tag.

Thanks! It worked.

jayanthvn · 2022-01-10T12:34:02Z

@cgchinmay - Can you update your branch? We can finish the review this week.

cmd/cni-metrics-helper/README.md

jayanthvn

Just one minor nit, rest lgtm.

cgchinmay requested review from jayanthvn, achevuru and srini-ram October 28, 2021 22:52

achevuru reviewed Oct 29, 2021

View reviewed changes

cmd/cni-metrics-helper/metrics/metrics.go Outdated Show resolved Hide resolved

achevuru reviewed Oct 29, 2021

View reviewed changes

cmd/cni-metrics-helper/metrics/metrics.go Outdated Show resolved Hide resolved

achevuru reviewed Oct 29, 2021

View reviewed changes

pkg/publisher/publisher.go Outdated Show resolved Hide resolved

cgchinmay force-pushed the fix_imds_access_cw branch from 2732843 to 2d7f32d Compare November 1, 2021 22:43

jayanthvn reviewed Nov 2, 2021

View reviewed changes

config/master/cni-metrics-helper.yaml Show resolved Hide resolved

jayanthvn requested changes Nov 2, 2021

View reviewed changes

jayanthvn added this to the v1.10.1 milestone Nov 2, 2021

srini-ram requested review from jayanthvn and achevuru November 4, 2021 05:16

jayanthvn reviewed Nov 4, 2021

View reviewed changes

config/master/manifests.jsonnet Show resolved Hide resolved

achevuru reviewed Nov 7, 2021

View reviewed changes

pkg/publisher/publisher.go Show resolved Hide resolved

achevuru reviewed Nov 8, 2021

View reviewed changes

pkg/publisher/publisher.go Outdated Show resolved Hide resolved

cgchinmay force-pushed the fix_imds_access_cw branch from 0f986b5 to f5eed93 Compare November 8, 2021 06:28

cgchinmay force-pushed the fix_imds_access_cw branch from dd0ae62 to f7f15e5 Compare November 9, 2021 16:16

cgchinmay requested a review from jayanthvn November 9, 2021 20:06

cgchinmay requested a review from a team as a code owner November 12, 2021 23:13

cgchinmay force-pushed the fix_imds_access_cw branch from 4343997 to f2f7709 Compare November 12, 2021 23:18

cgchinmay force-pushed the fix_imds_access_cw branch from b1ff807 to 32a756a Compare December 9, 2021 22:13

cgchinmay force-pushed the fix_imds_access_cw branch from 60dafc0 to e394298 Compare December 9, 2021 23:47

cgchinmay added 2 commits December 13, 2021 07:45

Minor change to logging CLUSTER_ID and Region values

5e7cd2a

Fix for cni-metrics-helper failing integration test

Fixed merge conflicts

80376f9

cgchinmay mentioned this pull request Dec 13, 2021

Changes for rc image v1.10.2-rc1 [VlanID and cni-metrics-helper] #1792

Merged

jayanthvn mentioned this pull request Dec 14, 2021

Cannot run metrics helper with IAM roles mapped to k8s service accounts #663

Closed

Readme update

af78032

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

3590647

cgchinmay mentioned this pull request Dec 20, 2021

Published docker images are vunerable to ALAS-2021-1552 #1774

Closed

cgchinmay added 3 commits January 18, 2022 21:47

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

f6c02ba

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

ae84e78

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

4992a4c

jayanthvn reviewed Jan 27, 2022

View reviewed changes

cmd/cni-metrics-helper/README.md Outdated Show resolved Hide resolved

jayanthvn approved these changes Jan 27, 2022

View reviewed changes

cgchinmay added 4 commits January 26, 2022 23:55

Updated Readme with more description of AWS_CLUSTER_ID

2ca1315

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

2b2ea0b

Merge remote-tracking branch 'upstream/master' into fix_imds_access_cw

c0c4a6c

minor change

f2035b1

cgchinmay merged commit 852d811 into aws:master Jan 27, 2022

cgchinmay mentioned this pull request Jan 28, 2022

1.10.2 changes #1830

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch Region and CLUSTER_ID information from cni-metrics-helper env #1715

Fetch Region and CLUSTER_ID information from cni-metrics-helper env #1715

cgchinmay commented Oct 28, 2021 •

edited

jayanthvn left a comment

jayanthvn commented Nov 2, 2021

cgchinmay commented Nov 9, 2021

groodt commented Jan 10, 2022

cgchinmay commented Jan 10, 2022 •

edited

groodt commented Jan 10, 2022

cgchinmay commented Jan 10, 2022

groodt commented Jan 10, 2022

jayanthvn commented Jan 10, 2022

jayanthvn left a comment

Fetch Region and CLUSTER_ID information from cni-metrics-helper env #1715

Fetch Region and CLUSTER_ID information from cni-metrics-helper env #1715

Conversation

cgchinmay commented Oct 28, 2021 • edited

jayanthvn left a comment

Choose a reason for hiding this comment

jayanthvn commented Nov 2, 2021

cgchinmay commented Nov 9, 2021

groodt commented Jan 10, 2022

cgchinmay commented Jan 10, 2022 • edited

groodt commented Jan 10, 2022

cgchinmay commented Jan 10, 2022

groodt commented Jan 10, 2022

jayanthvn commented Jan 10, 2022

jayanthvn left a comment

Choose a reason for hiding this comment

cgchinmay commented Oct 28, 2021 •

edited

cgchinmay commented Jan 10, 2022 •

edited