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

fix(environments): buildah cannot find base images. #548

Merged
merged 2 commits into from
May 16, 2023

Conversation

simbuerg
Copy link
Member

@simbuerg simbuerg commented Mar 7, 2023

Maybe this fixes #544

@simbuerg simbuerg requested a review from boehmseb March 7, 2023 00:20
@@ -400,7 +400,7 @@ def _create(self, tag: str, from_: model.FromLayer) -> model.MaybeContainer:
image = model.Image(tag, from_, [])

# container_id, err = run(bb_buildah('from')[from_.base])
res = run(bb_buildah('from')[from_.base])
res = run(bb_buildah('from')[from_.base.lower()])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it make sense to use the existing conversion function oci_compliant_name() from commands.py?

I can't say for sure whether this is the correct location to apply this transformation since I don't know whether this is the only location where FromLayers are handled.
My first idea would have been to use a converter in the FromLayer class as is done with the commands, i.e.,

class FromLayer(Layer):
    base: str = attr.ib(converter=oci_compliant_name)

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. For this I just used the same behavior as in other locations in the same module. Using oci_compliant_name is the sane way to go.

@boehmseb
Copy link
Collaborator

@simbuerg this should be ready to merge, right?

@simbuerg simbuerg merged commit 1e69a6c into master May 16, 2023
@simbuerg simbuerg deleted the simbuerg/fix-544-buildah-cannot-find-image branch May 16, 2023 18:43
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.

ContainerImage.from_ does not normalize base image name with oci_compliant_name()
2 participants