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

Use ECR feed for OCI and docker #1415

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

eddymoulton
Copy link
Contributor

@eddymoulton eddymoulton commented Dec 17, 2024

ECR supports more than just container (docker) images as it's a fully featured OCI registry.

To allow support for this we can query the manifest before we download/pull the image to switch between our pre-existing support for OCI feeds and docker feeds.

This approach means that users must have some pre-existing knowledge about the package when selecting from an ECR feed as it could be a helm chart, container image or any other OCI artifact type.
It is worth nothing that currently we do nothing to filter out non-docker images from the search and any helm charts in ECR would appear as options when deploying a container - but would simply fail to deploy.

I was going to put this behind a feature toggle, but when we try to run the acquire packages step we don't pass any of those variables down to Calamari and it runs with the feature toggle off - which grabs the docker package instead.
This is fully crashing on my machine, but I think for unrelated reasons. I am going to test the same on a branch instance to find out more.

[[sc-75291]]

Comment on lines +30 to +39
var isHelmChart =
jsonManifest.HasConfigMediaTypeContaining(OciConstants.Manifest.Config.OciImageMediaTypeValue)
|| jsonManifest.HasLayersMediaTypeContaining(OciConstants.Manifest.Layers.HelmChartMediaTypeValue);

if (isHelmChart)
return OciArtifactTypes.HelmChart;

var isDockerImage = jsonManifest.HasMediaTypeContaining(OciConstants.Manifest.DockerImageMediaTypeValue)
|| jsonManifest.HasConfigMediaTypeContaining(OciConstants.Manifest.Config.DockerImageMediaTypeValue)
|| jsonManifest.HasLayersMediaTypeContaining(OciConstants.Manifest.Layers.DockerImageMediaTypeValue);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for myself before merging, I need to confirm these are all correct and expected.

@eddymoulton eddymoulton marked this pull request as draft December 19, 2024 04:56
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.

2 participants