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

Push container images to GHCR #2576

Merged
merged 6 commits into from Oct 1, 2021

Conversation

evan2645
Copy link
Member

This commit updates our GitHub workflows to push container images to
GHCR in addition to GCR. Assuming all goes well, we will deprecate
GCR-based images in a futur release.

One major change here is that this commit ships scratch images to GHCR
instead of alpine-based images. Switching base images has been something
we've discussed for a while now, and now seems like the right time to do
it. GCR will still receive alpine images.

Signed-off-by: Evan Gilman egilman@vmware.com

This commit updates our GitHub workflows to push container images to
GHCR in addition to GCR. Assuming all goes well, we will deprecate
GCR-based images in a futur release.

One major change here is that this commit ships scratch images to GHCR
instead of alpine-based images. Switching base images has been something
we've discussed for a while now, and now seems like the right time to do
it. GCR will still receive alpine images.

Signed-off-by: Evan Gilman <egilman@vmware.com>

echo "Pushing images tagged as $IMAGETAG..."

for img in spire-server spire-agent; do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notably missing here is k8s-workload-registrar and oidc-discovery-provider.

The GHCR repo is namespaced under the org rather than the repo, unfortunately. This means we need to be extra careful when selecting image names ... they should clearly reflect their purpose and make sense under the larger SPIFFE scope. In my opinion, these two names do not meet that requirement.

For the k8s-workload-registrar ... we will soon have new spire-controller code being published as an image on GHCR, so I kind of feel like we should just roll with that. It is already namespaced appropriately.

For the oidc-discovery-provider.. it relies on SPIRE APIs. I'm not sure what's best ... maybe spire-oidc-provider? Or spire-oidc-discovery-provider?

For now I've left these two out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spire-oidc-provider seems ok?

@echo Building $1...
$(E)$(go_path) CGO_ENABLED=0 go build $$(go_flags) -ldflags $$(go_ldflags) -o $1 $2

endef
# https://7thzero.com/blog/golang-w-sqlite3-docker-scratch-image
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it still makes sense to keep this reference here or not ... it feels old but there's no year on the post

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be ok removing it 🤷

for img in spire-server spire-agent; do
ghcrimg=ghcr.io/"$ORGNAME"/"$img":"${IMAGETAG}"
echo "Executing: docker tag $img:latest-local $ghcrimg"
docker tag "$img":latest-local "$ghcrimg"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be tagging the scratch image?

Suggested change
docker tag "$img":latest-local "$ghcrimg"
docker tag "$img"-scratch:latest-local "$ghcrimg"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to switch the "default" image to scratch, and that's all we ship. The move to GHCR acts as the deprecation function.

Should we continue to ship both flavors? The only argument I can see is some sort of convenience or dev image, in which case it should probably be named as such (and the non-suffixed version should still be scratch)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to switch the "default" image to scratch, and that's all we ship. The move to GHCR acts as the deprecation function.

That's what I figured, but I didn't see any changes in the Makefile that changed way things are tagged and the images.tar.gz loaded by this step on the workflow contains the non-scratch images.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I figured, but I didn't see any changes in the Makefile that changed way things are tagged and the images.tar.gz loaded by this step on the workflow contains the non-scratch images.

Aah yes, thanks. I will fix that.

Signed-off-by: Evan Gilman <egilman@vmware.com>
Signed-off-by: Evan Gilman <egilman@vmware.com>
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, provided we can get consensus on the k8s-workload-registrar and oidc-discovery-provider. Thanks for looking into this, @evan2645 !

Signed-off-by: Evan Gilman <egilman@vmware.com>
Signed-off-by: Evan Gilman <egilman@vmware.com>
@evan2645
Copy link
Member Author

evan2645 commented Oct 1, 2021

I updated this PR to also push spire-oidc-provider.

One complication is that now this no longer matches the image name we build and test against, so I had to special case it in the image push script. I left a comment there to switch everything over when we deprecate GCR ... not sure if there's a better option, open to suggestions

@azdagron azdagron merged commit e675f69 into spiffe:main Oct 1, 2021
@evan2645 evan2645 deleted the push-scratch-images-to-ghcr branch October 1, 2021 23:00
@azdagron azdagron added this to the 1.1.0 milestone Oct 4, 2021
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.

None yet

2 participants