Expand build argument from environment when no value specified #993

antechrestos · 2020-01-23T17:16:56Z

Fixes #713

Description

This change adds the feature of getting value of build argument from environment when build argument is specified as follows

/kaniko/executor .... --build-arg MY_ARGUMENT ...

Submitter Checklist

Includes unit tests
Adds integration tests if needed.

Reviewer Notes

The code flow looks good.
Unit tests and or integration tests added.

Release Notes

Allow user to provide build argument from environment variable as follows:
$> export MY_ARGUMENT="the value"
$> /kaniko/executor ... --build-arg MY_ARGUMENT ...
Or
$> docker run -e MY_ARGUMENT="the value" gcr.io/kaniko-project/executor ... --build-arg MY_ARGUMENT ...

antechrestos · 2020-01-23T17:17:34Z

@cvgw As promised... Do I need to add integration test?

tejal29 · 2020-01-24T22:07:43Z

Thank you so much for your contribution!

cvgw

I'm not sure this addresses the primary use case which is secrets

won't docker -e SOME_SECRET=secret-value run .... output the value of the secret to the log/shell history?

I think we need a solution that won't be logged out. We also need to audit the logs in kaniko and make sure this value will never be logged. Infact, we should probably write an integration tests that checks the logs to make sure a known secret value is never logged

stewi2 · 2020-01-25T02:48:15Z

won't docker -e SOME_SECRET=secret-value run .... output the value of the secret to the log/shell history?

At least that part can be solved via a .env file. I.e.

docker --env-file .secrets run ....

with .secrets containing this:

SOME_SECRET=secret-value

Granted, that file would need to be written to disk first, but this is a common way to pass along environment variables without exposing them via the command line.

antechrestos · 2020-01-25T22:08:26Z

@tejal29 you're welcome
@cvgw I added it. To do so I added a new dedicated dockerfile. I also added a way to pass environment variable to build (with cmd.Env for docker build and -e for kaniko build as kaniko build is done inside docker) and also tested the expansion of docker variable.
For this test a check of output is provided te do a research on argument value and ensure that it is not printed.

cvgw · 2020-01-27T00:00:21Z

cmd/executor/cmd/root.go

@@ -206,6 +207,17 @@ func resolveDockerfilePath() error {
 	return errors.New("please provide a valid path to a Dockerfile within the build context with --dockerfile")
 }

+// resolveEnvironmentBuildArgs replace build args without value by the same named environment variable
+func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) string) {


So this will take an argument like FOO and replace it with FOO=FOO ?

@cvgw this will take the argument array such as [ "foo=bar", "EnvVariable" ] crawl through it and replace any argument without = with "EnvVariable=EnvValue" if a value was returned by resolver function (os.Getenv in prod code, mocked function in tests). If zero length value returned, it will be replaced with "EnvVariable="

Thanks for explaining, cheers

cvgw · 2020-01-27T00:03:37Z

integration/images.go

+
+// Checks if argument are not printed in output.
+// Argument may be passed through --build-arg key=value manner or --build-arg key with key in environment
+func checkArgsNotPrinted(dockerfile string, out []byte) error {


So this checks to make sure the value of none of the build args are logged? It makes no distinction between whether they were added as literals or expanded from the env?

@cvgw I could have make the distinction in the test, however I could not find any way to make the distinction in the code as env like variable (env, arg and meta arg ) are used everywhere as a mere string key=value and could not make the distinction without breaking everything.
Do you want I change the test? Or everything?

No changes needed. Was just clarifying for my owner understanding. I think it's reasonable to not log any ARG values.

cvgw

This looks really nice, just a couple questions

antechrestos

@cvgw Thank you for the feed back. I answered .

antechrestos · 2020-01-27T08:10:18Z

cmd/executor/cmd/root.go

@@ -206,6 +207,17 @@ func resolveDockerfilePath() error {
 	return errors.New("please provide a valid path to a Dockerfile within the build context with --dockerfile")
 }

+// resolveEnvironmentBuildArgs replace build args without value by the same named environment variable
+func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) string) {


@cvgw this will take the argument array such as [ "foo=bar", "EnvVariable" ] crawl through it and replace any argument without = with "EnvVariable=EnvValue" if a value was returned by resolver function (os.Getenv in prod code, mocked function in tests). If zero length value returned, it will be replaced with "EnvVariable="

cvgw · 2020-01-27T17:13:20Z

cmd/executor/cmd/root.go

@@ -206,6 +207,17 @@ func resolveDockerfilePath() error {
 	return errors.New("please provide a valid path to a Dockerfile within the build context with --dockerfile")
 }

+// resolveEnvironmentBuildArgs replace build args without value by the same named environment variable
+func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) string) {


Thanks for explaining, cheers

antechrestos · 2020-01-28T09:09:05Z

@cvgw something went wrong with kaniko. However, things went well in Travis 🤔... What went wrong?

tejal29 · 2020-01-28T21:25:33Z

@antechrestos i triggered a rebuilt. Previous failure was a flake.

antechrestos · 2020-01-28T21:31:09Z

@tejal29 thanks

antechrestos · 2020-01-29T08:43:41Z

@tejal29 I have rebased my branch. I am not sure it might change anything 🤔

Fixes #713

antechrestos · 2020-01-30T23:15:42Z

@cvgw @tejal29 may I have another try with kokoro? 🙏

tejal29 · 2020-01-31T19:26:52Z

@antechrestos The travis test passed, so i am going to merge this in!

antechrestos · 2020-01-31T20:03:26Z

@tejal29 at our own risk 😊

sourcec0de · 2020-02-21T02:48:49Z

THANK YOU! ❤️

nhed · 2023-02-09T02:58:59Z

Thanks this is so helpful (albeit undocumented) feature

googlebot added the cla: yes CLA signed by all commit authors label Jan 23, 2020

tejal29 approved these changes Jan 24, 2020

View reviewed changes

tejal29 added the candidate-release label Jan 24, 2020

cvgw suggested changes Jan 25, 2020

View reviewed changes

antechrestos requested a review from cvgw January 25, 2020 22:04

cvgw reviewed Jan 27, 2020

View reviewed changes

antechrestos commented Jan 27, 2020

View reviewed changes

cvgw approved these changes Jan 27, 2020

View reviewed changes

cvgw added the kokoro:run label Jan 27, 2020

kokoro-team removed the kokoro:run label Jan 27, 2020

Expand build argument from environment when no value specified

2f1e54e

Fixes #713

tejal29 merged commit de4b734 into GoogleContainerTools:master Jan 31, 2020

antechrestos deleted the feature/expand_build_args_with_environment_variable branch January 31, 2020 20:04

iqbalaydrus mentioned this pull request May 24, 2023

Passing secretEnv to kaniko cloud build #2368

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand build argument from environment when no value specified #993

Expand build argument from environment when no value specified #993

antechrestos commented Jan 23, 2020 •

edited by tejal29

Loading

antechrestos commented Jan 23, 2020

tejal29 commented Jan 24, 2020

cvgw left a comment •

edited

Loading

stewi2 commented Jan 25, 2020

antechrestos commented Jan 25, 2020

cvgw Jan 27, 2020 •

edited

Loading

antechrestos Jan 27, 2020

cvgw Jan 27, 2020

cvgw Jan 27, 2020

antechrestos Jan 27, 2020

cvgw Jan 27, 2020

cvgw left a comment

antechrestos left a comment

antechrestos Jan 27, 2020

cvgw Jan 27, 2020

antechrestos commented Jan 28, 2020 •

edited

Loading

tejal29 commented Jan 28, 2020

antechrestos commented Jan 28, 2020

antechrestos commented Jan 29, 2020

antechrestos commented Jan 30, 2020

tejal29 commented Jan 31, 2020

antechrestos commented Jan 31, 2020

sourcec0de commented Feb 21, 2020

nhed commented Feb 9, 2023

Expand build argument from environment when no value specified #993

Expand build argument from environment when no value specified #993

Conversation

antechrestos commented Jan 23, 2020 • edited by tejal29 Loading

antechrestos commented Jan 23, 2020

tejal29 commented Jan 24, 2020

cvgw left a comment • edited Loading

Choose a reason for hiding this comment

stewi2 commented Jan 25, 2020

antechrestos commented Jan 25, 2020

cvgw Jan 27, 2020 • edited Loading

Choose a reason for hiding this comment

antechrestos Jan 27, 2020

Choose a reason for hiding this comment

cvgw Jan 27, 2020

Choose a reason for hiding this comment

cvgw Jan 27, 2020

Choose a reason for hiding this comment

antechrestos Jan 27, 2020

Choose a reason for hiding this comment

cvgw Jan 27, 2020

Choose a reason for hiding this comment

cvgw left a comment

Choose a reason for hiding this comment

antechrestos left a comment

Choose a reason for hiding this comment

antechrestos Jan 27, 2020

Choose a reason for hiding this comment

cvgw Jan 27, 2020

Choose a reason for hiding this comment

antechrestos commented Jan 28, 2020 • edited Loading

tejal29 commented Jan 28, 2020

antechrestos commented Jan 28, 2020

antechrestos commented Jan 29, 2020

antechrestos commented Jan 30, 2020

tejal29 commented Jan 31, 2020

antechrestos commented Jan 31, 2020

sourcec0de commented Feb 21, 2020

nhed commented Feb 9, 2023

antechrestos commented Jan 23, 2020 •

edited by tejal29

Loading

cvgw left a comment •

edited

Loading

cvgw Jan 27, 2020 •

edited

Loading

antechrestos commented Jan 28, 2020 •

edited

Loading