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

NAME and VERSION leak into toolbox container env #188

Closed
juhp opened this issue Jun 7, 2019 · 16 comments
Closed

NAME and VERSION leak into toolbox container env #188

juhp opened this issue Jun 7, 2019 · 16 comments
Milestone

Comments

@juhp
Copy link
Contributor

juhp commented Jun 7, 2019

I noticed that envvar for ENV NAME=fedora-toolbox VERSION=30 leak into the toolbox environment.
I am not sure if this is desirable: eg NAME affects changelog entry generation in Emacs rpm-spec-mode. And probably having VERSION in the container is not that useful either.

@debarshiray
Copy link
Member

Yeah, those variables are exported by Podman. It always felt odd that they aren't namespaced. I wonder if Docker does the same, and so it's done for compatibility.

@debarshiray
Copy link
Member

I wish I could transfer this to libpod. Doesn't look like I have the necessary permissions for that. :(

@juhp could you please (re)file this against libpod ? We could do something to suppress these variables, but they come directly from Podman, so might be better to at least discuss it a bit first. I have no idea why such generically named and non-namespaced variables were chosen in the first place.

@debarshiray
Copy link
Member

Oh wait, they actually come from the Dockerfile for the fedora-toolbox images.

@debarshiray
Copy link
Member

They are mandated by the Fedora guidelines.

@debarshiray
Copy link
Member

On closer reading, it doesn't look like the guidelines actually require them, but they are part of the example Dockerfile snippets.

@cverna
Copy link

cverna commented Jan 10, 2020

So from a quick look at OSBS it seems that these labels are used to populate some data in koji post build. I don't think we can remove them.

I ll look into more details next week if there is a work around.

@debarshiray
Copy link
Member

Thanks for clarifying that, @cverna !

@debarshiray
Copy link
Member

@cverna confirmed in #fedora-containers on Freenode that NAME and VERSION really do need to be present in the Dockerfile.

@HarryMichal
Copy link
Member

@debarshiray, I guess we could unset the env vars when the container is being initialized. That way we keep them in the Dockerfiles but they will not cause problems during runtime.

@HarryMichal HarryMichal reopened this Jun 13, 2020
@Jmennius
Copy link
Collaborator

@debarshiray Can you clarify why exactly are they required?
Should we have them in non-Fedora images?

@debarshiray
Copy link
Member

I guess we could unset the env vars when the container is
being initialized. That way we keep them in the Dockerfiles
but they will not cause problems during runtime.

The NAME and VERSION environment variables make their way into the toolbox containers from their corresponding images, and are part of the containers' metadata. See:

$ podman inspect --type container <container>

We could somehow unset the variables right before spawning the interactive shell inside the container. However, that would deepen the voodoo that we perform when launching the shell.

A cleaner alternative might be to block the environment variables from getting leaked into the container from the image, but I don't see an obvious way to do that.

@debarshiray
Copy link
Member

Can you clarify why exactly are they required?

As @cverna mentioned, they are needed by the Fedora build system.

Should we have them in non-Fedora images?

I don't think you need them in non-Fedora images.

@Jmennius
Copy link
Collaborator

Thanks for the answer.
It looks like the only clean solution would be to change OSBS to rely on labels (which are actually designed for this type of metadata).

@debarshiray
Copy link
Member

Closing.

We can revisit this once the Fedora build system no longer needs NAME and VERSION.

@JayDoubleu
Copy link

JayDoubleu commented Jan 16, 2022

@debarshiray I might be wrong on this but by looking at the guidelines, looks like there is a requirement for the right labels to be present by fedora build system and not the env vars themeselves ?

image
https://github.com/containerbuildsystem/atomic-reactor/blob/bdf5ecf52db1a30a5db135d12b51a488c04ebe29/atomic_reactor/plugins/pre_bump_release.py#L253..L266

Could the env vars in Containerfile be converted to ARGS instead or change them to TOOLBOX_NAME and TOOLBOX_VERSION which would be quite useful to have inside?

@debarshiray
Copy link
Member

Could the env vars in Containerfile be converted to ARGS instead

Just now @travier brought this up in containers/docs#15

Unfortunately, I am not an expert in Fedora's OCI image building pipeline. I
mostly follow what the maintainers of the pipeline mandate. :)

debarshiray pushed a commit to debarshiray/toolbox that referenced this issue Mar 1, 2023
Note that there can be only one ARG per line.  Otherwise, the build may
fail with some build systems.  eg., Fedora's [1], which uses Docker, not
Podman.

Only the images for currently maintained Fedoras (ie., 36, 37, 38 & 39)
were updated.

[1] https://koji.fedoraproject.org/koji/taskinfo?taskID=98150241

containers#188
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 a pull request may close this issue.

6 participants