Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: pull s3 binaries as root #2058

Merged
merged 1 commit into from
Nov 19, 2024
Merged

build: pull s3 binaries as root #2058

merged 1 commit into from
Nov 19, 2024

Conversation

ndbaker1
Copy link
Member

Issue #, if available:

Description of changes:

Whenever the AMI builds pull from S3 they no longer use act as root (like all other operations). This change runs the aws s3 cp steps with sudo -E to preserve the AWS creds from the environment.

this affects kubelet for example, which you would expect to be owned by root

sh-5.2$ cat /etc/eks/release
BASE_AMI_ID="ami-0c968d9bc191231c8"
BUILD_TIME="Sat Nov  9 23:07:34 UTC 2024"
BUILD_KERNEL="6.1.112-124.190.amzn2023.x86_64"
ARCH="x86_64"
sh-5.2$ ls -latr /usr/bin/kubelet
-rwxrwxr-x. 1 ec2-user ec2-user 76931672 Sep 17 19:16 /usr/bin/kubelet

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

results for a rebuild ami:

sh-5.2$ cat /etc/eks/release
BASE_AMI_ID="ami-0f8bdccd767e6b0c6"
BUILD_TIME="Sun Nov 17 00:37:10 UTC 2024"
BUILD_KERNEL="6.1.115-126.197.amzn2023.x86_64"
ARCH="x86_64"
sh-5.2$ ls -latr /usr/bin/kubelet
-rwxr-xr-x. 1 root root 76931672 Sep 18 17:49 /usr/bin/kubelet

See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.

@ndbaker1
Copy link
Member Author

/ci

Copy link
Contributor

@ndbaker1 roger that! I've dispatched a workflow. 👍

Copy link
Contributor

@ndbaker1 the workflow that you requested has completed. 🎉

AMI variantBuildTest
1.24 / al2success ✅success ✅
1.24 / al2023success ✅success ✅
1.25 / al2success ✅success ✅
1.25 / al2023success ✅success ✅
1.26 / al2success ✅success ✅
1.26 / al2023success ✅success ✅
1.27 / al2success ✅success ✅
1.27 / al2023success ✅success ✅
1.28 / al2success ✅success ✅
1.28 / al2023success ✅success ✅
1.29 / al2failure ❌skipped ⏭️
1.29 / al2023success ✅success ✅
1.30 / al2success ✅success ✅
1.30 / al2023success ✅success ✅
1.31 / al2success ✅success ✅
1.31 / al2023success ✅success ✅

@ndbaker1
Copy link
Member Author

ndbaker1 commented Nov 17, 2024

1.29 failure isn't related to the changes, seems like just some flake in one of the actions after the ami step

@ndbaker1 ndbaker1 merged commit 67568ec into awslabs:main Nov 19, 2024
10 checks passed
@ndbaker1 ndbaker1 deleted the build-root branch November 19, 2024 00:26
@cartermckinnon
Copy link
Member

@ndbaker1 chown seems like a better approach for this, if the goal is just file ownership

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants