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.

Sign up for GitHub

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

Jump to bottom

implement env variable expansion for kaniko builds #3974

Closed
wants to merge 2 commits into from
Closed

implement env variable expansion for kaniko builds #3974

wants to merge 2 commits into from

Conversation

DanielSel
Copy link
Contributor

Adds environment variable expansion for kaniko builds (to address currently inconsistent behaviour between buildArgs and envs).

@codecov
Copy link

codecov bot commented Apr 17, 2020 •
edited
Loading

Codecov Report

Merging #3974 into master will decrease coverage by 0.04%.
The diff coverage is 68.96%.

Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100.00% <ø> (ø)
pkg/skaffold/build/cluster/kaniko.go 35.78% <65.38%> (+35.78%) ⬆️
pkg/skaffold/build/cluster/pod.go 86.90% <100.00%> (ø)
pkg/skaffold/docker/image.go 75.00% <100.00%> (+0.33%) ⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go 65.85% <0.00%> (-2.44%) ⬇️

@pull-request-size pull-request-size bot added size/L and removed size/S labels Apr 18, 2020
tejal29
tejal29 requested changes Apr 23, 2020
View reviewed changes
Copy link
Member

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Needs more explanation

pkg/skaffold/build/cluster/kaniko.go Show resolved Hide resolved
@tejal29 tejal29 self-assigned this Apr 23, 2020
This was referenced Apr 27, 2020
Merged
Merged
@DanielSel DanielSel force-pushed the feature/kaniko-env-expansion branch from 310d5a6 to 2c532db Compare April 27, 2020 11:56
@DanielSel DanielSel force-pushed the feature/kaniko-env-expansion branch from 2c532db to a199ac8 Compare April 27, 2020 12:24
@DanielSel DanielSel mentioned this pull request Apr 27, 2020
Closed
@DanielSel DanielSel requested a review from tejal29 April 27, 2020 13:52
@DanielSel
Copy link
Contributor Author

@tejal29 I think the CI re-build is stuck

@gsquared94
Copy link
Collaborator

@DanielSel It seems that Travis is stuck. You can push an empty commit to force it to trigger another job. That worked for me once before.

@tejal29 tejal29 mentioned this pull request Jun 17, 2020
Closed
@tejal29
Copy link
Member

tejal29 commented Jun 18, 2020

@DanielSel you need to run make generate-schema on this branch.
Can you please do that?

@tejal29 tejal29 modified the milestone: v1.12.0 Jun 18, 2020
@DanielSel
Copy link
Contributor Author

Hey guys,
sorry I was unable to access my computer for quite a while now. I will do it as soon as I can find the time. Thank you!

@gsquared94
Copy link
Collaborator

@DanielSel np, I'll close the PR for now. Please reopen once you've made the change.

@gsquared94 gsquared94 closed this Jul 6, 2020
@DanielSel DanielSel mentioned this pull request Jul 24, 2020
Merged
@DanielSel
Copy link
Contributor Author

I made the change and re-submitted the PR here: #4557

joanfabregat
joanfabregat reviewed Feb 22, 2022
View reviewed changes
pkg/skaffold/build/cluster/kaniko_test.go
if err != nil {
t.Fatalf("error processing generated env variables from image uri: %s", err)
}
env, err := evaluateEnv(artifact.Env, generatedEnvs...)
Copy link

@joanfabregat joanfabregat Feb 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The documentation in v2beta3.json states that the env vars are available for buildArgs: but it seems that is it only the case for env:. @DanielSel Should I report a bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Awem Awem Awem left review comments

@joanfabregat joanfabregat joanfabregat left review comments

@balopat balopat Awaiting requested review from balopat

@dgageot dgageot Awaiting requested review from dgageot

@nkubala nkubala Awaiting requested review from nkubala

@briandealwis briandealwis Awaiting requested review from briandealwis

@tejal29 tejal29 Awaiting requested review from tejal29

Assignees

@tejal29 tejal29

Labels
cla: yes size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DanielSel @gsquared94 @tejal29 @Awem @joanfabregat @googlebot