fix: make govcloud builds possible#2621
Conversation
fletcherw
force-pushed
the
build_credentials
branch
from
9bdbd5a to
9ac7933
Compare
|
/ci |
|
@fletcherw roger that! I've dispatched a workflow. 👍 |
| REGION="${2}" | ||
|
|
||
| CROSS_PARTITION_ARGS="" | ||
| if [[ "${REGION}" == us-gov* || "${REGION}" == eusc* ]]; then |
There was a problem hiding this comment.
I'd like this to be more tied to connectivity. if it's not an isolated partition or China, we know we can allow the cross-partition
but I recognize this is a hack script and it's not so easy to make this determination here
There was a problem hiding this comment.
I see your point but I think it's okay like this in the hack script.
hack/latest-binaries.sh
Outdated
| MINOR_VERSION="${1}" | ||
| REGION="${2}" | ||
|
|
||
| CROSS_PARTITION_ARGS="" |
There was a problem hiding this comment.
nit: this name seems awkward in the invocation when it's not cross-partition, maybe something more generic like S3_CLI_EXTRA_ARGS
| "nvidia_repository_url": null, | ||
| "nvidia_grid_runfile_bucket_name": "ec2-linux-nvidia-drivers", | ||
| "pause_container_image": "602401143452.dkr.ecr.us-west-2.amazonaws.com/eks/pause:3.10", | ||
| "pause_container_image": "public.ecr.aws/eks-distro/kubernetes/pause:3.10", |
There was a problem hiding this comment.
also, we should fix the caching of this image - it's a different auth token and subcommand for ecr-public
|
@fletcherw the workflow that you requested has completed. 🎉
|
fletcherw
force-pushed
the
build_credentials
branch
from
9ac7933 to
136e6b2
Compare
|
1.33 test failure was just a timeout. |
fletcherw
force-pushed
the
build_credentials
branch
from
136e6b2 to
488c452
Compare
Make various fixes so that govcloud builds work: * Make latest-binaries.sh handle fetching cross-partition * Change s3 copy logic to not check the AWS_ACCESS_KEY_ID variable, which is a fundamentally flawed check, and instead just attempt a copy using the environment's credentials and fall back to --no-sign-request if the initial copy fails. * update the cache-pause-container script to use the eks-distro public mirror The net effect of this change is that by default, none of the provisioners rely on the caller's AWS credentials. I'm leaving in the logic that passes through the AWS credentials via environment variables since removing is a breaking change, but none of them are needed. In a future major release, it would be nice to remove those entirely and require people to use instance roles if they need permissions from the provisioners (e.g. for private artifact buckets).
fletcherw
force-pushed
the
build_credentials
branch
from
488c452 to
807267a
Compare
|
@fletcherw is the real MVP! |
|
Thanks @bradwatsonaws ! Please let me know if you have any issues with the change. |
3 participants
Make various fixes so that govcloud builds work:
The net effect of this change is that by default, none of the provisioners rely on the caller's AWS credentials. I'm leaving in the logic that passes through the AWS credentials via environment variables since removing is a breaking change, but none of them are needed.
In a future major release, it would be nice to remove those entirely and require people to use instance roles if they need permissions in the provisioners (e.g. for private artifact buckets).
Fixes: #527, #762, #1772
Testing:
Builds in us-gov-west-1 and us-gov-east-1 with AWS credential env vars unset.
Build in us-gov-west-1 with AWS credential env vars set.
k8s 1.29 and 1.35 builds in us-west-2 with AWS credential env vars unset.