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

add --with-env flag to publish command #12482

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

glours
Copy link
Contributor

@glours glours commented Jan 21, 2025

this flag allow publishing env variables in the Compose OCI artifact

What I did
Add --with-env flag to let user explicitly optin the publication of env variables in the OCI artifact

Related issue
https://docker.atlassian.net/browse/COMP-873

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@glours glours requested a review from a team as a code owner January 21, 2025 08:56
@glours glours requested review from ndeloof and aevesdocker January 21, 2025 08:56
@glours glours self-assigned this Jan 21, 2025
@glours
Copy link
Contributor Author

glours commented Jan 21, 2025

@aevesdocker can you check if the new error message is ok for you? 😇 🙏

@glours
Copy link
Contributor Author

glours commented Jan 21, 2025

@ndeloof I wonder if we should not just set all env variables to empty strings when publishing? 🤔
This way we'll push the env variables needed to run the application without sending sensitive data and we'll be able to ask consumers to set values when doing an docker compose -f oci://... up, wdyt?

docs/reference/compose_alpha_publish.md Outdated Show resolved Hide resolved
docs/reference/compose_alpha_publish.md Outdated Show resolved Hide resolved
this flag allow publishing env variables in the Compose OCI artifact

Signed-off-by: Guillaume Lours <[email protected]>
@glours glours force-pushed the add-with-env-flag-publish branch from beb234e to 72bde6f Compare January 21, 2025 09:11
" or remove sensitive data from your Compose configuration", service.Name)
}
if len(service.Environment) > 0 {
return fmt.Errorf("service %q has environment variable(s) declared. To avoid leaking sensitive data, "+
Copy link
Contributor

Choose a reason for hiding this comment

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

service.environment may be set with a fixed value, not relying on any interpolation. Typically:

db:
    image: mysql
    environment:
      MYSQL_DATABASE: avatar
      MYSQL_ROOT_PASSWORD_FILE: /run/secrets/db-password

those should not prevent compose file to be published, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact yes that should prevent from publishing by default because you can also have MYSQL_ROOT_PASSWORD: mySuperSecretPassword defined the same way

Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

I wonder the approach is correct.
IIUC the goal is to detect the compose file doesn't require env var to be set by user / risk user environment to leak. To prevent this, better load the compose model without any variable passed to interpolation, which will fail if some is needed.

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.

3 participants