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 "network" model option with "host" default value. #22

Merged
merged 4 commits into from
Jun 24, 2021

Conversation

silphid
Copy link
Owner

@silphid silphid commented Jun 16, 2021

Add "hostenv" option to pass host env vars as is.

Add "hostenv" option to pass host env vars as is.
@@ -17,9 +17,11 @@ type Context struct {
Image string
Build DockerBuild
Env map[string]string
HostEnv []string `yaml:"hostenv,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment meant for line 15: shouldn't it be yaml:"-" since the context name is not in the yaml file but built by us? Doesn't matter though but odd.

if source.Network != "" {
merged.Network = source.Network
}
merged.HostEnv = append(merged.HostEnv, source.HostEnv...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

dedup?

Base automatically changed from variants to master June 22, 2021 21:03
@silphid
Copy link
Owner Author

silphid commented Jun 22, 2021

Je sais qu'on avait discuté de supporter:

env:
    VAR1: $VAR1
    VAR2: $VAR2

mais, à court terme, j'ai plutôt opté pour la solution la plus simple et proche de la commande CLI --env VAR:

env:
    VAR1:
    VAR2:

On pourra plus tard ajouter le support pour l'extrapolation de variables avec $VAR et ${VAR}, mais ça nécessite plus d'investissement pour bien le faire (avec des regex et unit tests correspondants), alors que je ne vois pas le besoin pour l'instant. Si tu n'es pas d'accord, on pourra s'obstiner de vive voix! ;-P

@silphid silphid requested a review from davidmdm June 22, 2021 21:15
@davidmdm
Copy link
Collaborator

Your suggesting the the empty string gets mapped to the host env? I think we might as well just do the feature correctly. we basically only need to figure out one regex

@silphid silphid merged commit fb29086 into master Jun 24, 2021
@silphid silphid deleted the network-mode-and-host-env branch June 24, 2021 15:17
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