diff --git a/docs/builder_image.md b/docs/builder_image.md index 53cc661ca..3e954c020 100644 --- a/docs/builder_image.md +++ b/docs/builder_image.md @@ -15,8 +15,8 @@ final docker image, the three are: sources, scripts and builder image. During th build process sti must place sources and scripts inside that builder image. To do so sti creates a tar file containing the two and then streams that file into the builder image. Before executing `assemble` script, sti untars that file and places -its contents into the location specified with `--location` flag or `STI_LOCATION` -environment variable from the builder image (default location is `/tmp`). For this +its contents into the location specified with `--location` flag or `io.openshift.sti.location` +label from the builder image (default location is `/tmp`). For this to happen your image must supply tar archiving utility (command `tar` available in `$PATH`) and command line interpreter (command `/bin/sh`). Doing so will allow your image to use the fastest possible build path, because in all other cases when either diff --git a/docs/cli.md b/docs/cli.md index 8103b90ef..caf8fbcf6 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -105,9 +105,9 @@ All of these locations are checked on each build in the following order: 1. A script found at the `--scripts` URL 1. A script found in the application source `.sti/bin` directory -1. A script found at the default image URL (`STI_SCRIPTS_URL` environment variable) +1. A script found at the default image URL (`io.openshift.sti.scripts-url` label) -Both `STI_SCRIPTS_URL` environment variable specified in the image and `--scripts` flag +Both `io.openshift.sti.scripts-url` label specified in the image and `--scripts` flag can take one of the following form: * `image://path_to_scripts_dir` - absolute path inside the image to a directory where the STI scripts are located @@ -115,13 +115,13 @@ can take one of the following form: * `http(s)://path_to_scripts_dir` - URL to a directory where the STI scripts are located Additionally we allow specifying the location of both scripts and sources inside the image -prior executing the `assemble` script with `--location` flag or `STI_LOCATION` environment -variable set inside the image. The expected value of this flag is absolute and existing path +prior executing the `assemble` script with `--location` flag or `io.openshift.sti.location` +label set inside the image. The expected value of this flag is absolute and existing path inside the image, if none is specified the default value of `/tmp` is being used. In case of both of these specified the `--location` flag takes precedence over the environment variable. **NOTE**: In case where the scripts are already placed inside the image (using `--scripts` -or `STI_SCRIPTS_URL` with value `image:///path/in/image`) then this value applies only to +or `io.openshift.sti.scripts-url` with value `image:///path/in/image`) then this value applies only to sources and artifacts. #### Example Usage diff --git a/hack/build-test-images.sh b/hack/build-test-images.sh index 98f9a61f9..7efd1d66e 100755 --- a/hack/build-test-images.sh +++ b/hack/build-test-images.sh @@ -22,6 +22,7 @@ function buildimage() } buildimage sti_test/sti-fake test/integration/images/sti-fake +buildimage sti_test/sti-fake-env test/integration/images/sti-fake-env buildimage sti_test/sti-fake-user test/integration/images/sti-fake-user buildimage sti_test/sti-fake-scripts test/integration/images/sti-fake-scripts buildimage sti_test/sti-fake-scripts-no-save-artifacts test/integration/images/sti-fake-scripts-no-save-artifacts diff --git a/hack/test-integration.sh b/hack/test-integration.sh index 290f9fa0c..e3df45df7 100755 --- a/hack/test-integration.sh +++ b/hack/test-integration.sh @@ -17,7 +17,7 @@ set +e img_count=$(docker images | grep -c sti_test/sti-fake) set -e -if [ "${img_count}" != "5" ]; then +if [ "${img_count}" != "6" ]; then echo "You do not have necessary test images, be sure to run 'hack/build-test-images.sh' beforehand." exit 1 fi diff --git a/pkg/api/scripts.go b/pkg/api/scripts.go index f53329cc9..871a98f50 100644 --- a/pkg/api/scripts.go +++ b/pkg/api/scripts.go @@ -19,7 +19,7 @@ const ( const ( // UserScripts is the location of scripts downloaded from user provided URL (-s flag). UserScripts = "downloads/scripts" - // DefaultScripts is the location of scripts downloaded from default location (STI_SCRIPTS_URL environment variable). + // DefaultScripts is the location of scripts downloaded from default location (io.openshift.sti.scripts-url label). DefaultScripts = "downloads/defaultScripts" // SourceScripts is the location of scripts downloaded with application sources. SourceScripts = "upload/src/.sti/bin" diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go index a473b893a..7c7b0f33b 100644 --- a/pkg/docker/docker.go +++ b/pkg/docker/docker.go @@ -15,8 +15,13 @@ import ( ) const ( - ScriptsURL = "STI_SCRIPTS_URL" - Location = "STI_LOCATION" + ScriptsURLEnvironment = "STI_SCRIPTS_URL" + LocationEnvironment = "STI_LOCATION" + + ScriptsURLLabel = "io.openshift.sti.scripts-url" + LocationLabel = "io.openshift.sti.location" + + DefaultLocation = "/tmp" ) // Docker is the interface between STI and the Docker client @@ -195,8 +200,20 @@ func (d *stiDocker) RemoveContainer(id string) error { return d.client.RemoveContainer(opts) } +// getLabel gets label's value from the image metadata +func getLabel(image *docker.Image, name string) string { + if value, ok := image.Config.Labels[name]; ok { + return value + } + if value, ok := image.ContainerConfig.Labels[name]; ok { + return value + } + + return "" +} + // getVariable gets environment variable's value from the image metadata -func (d *stiDocker) getVariable(image *docker.Image, name string) string { +func getVariable(image *docker.Image, name string) string { envName := name + "=" env := append(image.ContainerConfig.Env, image.Config.Env...) for _, v := range env { @@ -208,21 +225,48 @@ func (d *stiDocker) getVariable(image *docker.Image, name string) string { return "" } -// GetScriptsURL finds a STI_SCRIPTS_URL in the given image's metadata +// GetScriptsURL finds a scripts-url label in the given image's metadata func (d *stiDocker) GetScriptsURL(image string) (string, error) { imageMetadata, err := d.CheckAndPull(image) if err != nil { return "", err } - scriptsURL := d.getVariable(imageMetadata, ScriptsURL) + return getScriptsURL(imageMetadata), nil +} + +// getScriptsURL finds a scripts url label in the image metadata +func getScriptsURL(image *docker.Image) string { + scriptsURL := getLabel(image, ScriptsURLLabel) if len(scriptsURL) == 0 { - glog.Warningf("Image does not contain a value for the STI_SCRIPTS_URL environment variable") + scriptsURL = getVariable(image, ScriptsURLEnvironment) + if len(scriptsURL) != 0 { + glog.Warningf("Image %s uses deprecated environment variable %s, please migrate it to %s label instead!", + image.ID, ScriptsURLEnvironment, ScriptsURLLabel) + } + } + if len(scriptsURL) == 0 { + glog.Warningf("Image does not contain a value for the %s label", ScriptsURLLabel) } else { - glog.V(2).Infof("Image contains STI_SCRIPTS_URL set to '%s'", scriptsURL) + glog.V(2).Infof("Image contains %s set to '%s'", ScriptsURLLabel, scriptsURL) } - return scriptsURL, nil + return scriptsURL +} + +// getLocation finds a location label in the image metadata +func getLocation(image *docker.Image) string { + if val := getLabel(image, LocationLabel); len(val) != 0 { + return val + } + if val := getVariable(image, LocationEnvironment); len(val) != 0 { + glog.Warningf("Image %s uses deprecated environment variable %s, please migrate it to %s label instead!", + image.ID, LocationEnvironment, LocationLabel) + return val + } + + // default directory if none is specified + return DefaultLocation } // RunContainer creates and starts a container using the image specified in the options with the ability @@ -245,12 +289,7 @@ func (d *stiDocker) RunContainer(opts RunContainerOptions) (err error) { // untar operation destination directory tarDestination := opts.Location if len(tarDestination) == 0 { - if val := d.getVariable(imageMetadata, Location); len(val) != 0 { - tarDestination = val - } else { - // default directory if none is specified - tarDestination = "/tmp" - } + tarDestination = getLocation(imageMetadata) } if opts.ExternalScripts { // for external scripts we must always append 'scripts' because this is @@ -261,7 +300,7 @@ func (d *stiDocker) RunContainer(opts RunContainerOptions) (err error) { // for internal scripts we can have separate path for scripts and untar operation destination scriptsURL := opts.ScriptsURL if len(scriptsURL) == 0 { - scriptsURL = d.getVariable(imageMetadata, ScriptsURL) + scriptsURL = getScriptsURL(imageMetadata) } commandBaseDir = strings.TrimPrefix(scriptsURL, "image://") glog.V(2).Infof("Base directory for STI scripts is '%s'. Untarring destination is '%s'.", diff --git a/pkg/docker/docker_test.go b/pkg/docker/docker_test.go index 5a9996eba..10045ae25 100644 --- a/pkg/docker/docker_test.go +++ b/pkg/docker/docker_test.go @@ -201,32 +201,34 @@ func TestGetScriptsURL(t *testing.T) { "not present": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{"Env1=value1"}, + Env: []string{"Env1=value1"}, + Labels: map[string]string{}, }, Config: &docker.Config{ - Env: []string{"Env2=value2"}, + Env: []string{"Env2=value2"}, + Labels: map[string]string{}, }, }, result: "", }, - "in containerConfig": { + "env in containerConfig": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{"Env1=value1", ScriptsURL + "=test_url_value"}, + Env: []string{"Env1=value1", ScriptsURLEnvironment + "=test_url_value"}, }, Config: &docker.Config{}, }, result: "test_url_value", }, - "in image config": { + "env in image config": { image: docker.Image{ ContainerConfig: docker.Config{}, Config: &docker.Config{ Env: []string{ "Env1=value1", - ScriptsURL + "=test_url_value_2", + ScriptsURLEnvironment + "=test_url_value_2", "Env2=value2", }, }, @@ -234,14 +236,24 @@ func TestGetScriptsURL(t *testing.T) { result: "test_url_value_2", }, - "contains =": { + "label in containerConfig": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{ScriptsURL + "=http://my.test.url/test?param=one"}, + Labels: map[string]string{ScriptsURLLabel: "test_url_value"}, }, Config: &docker.Config{}, }, - result: "http://my.test.url/test?param=one", + result: "test_url_value", + }, + + "label in image config": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{ + Labels: map[string]string{ScriptsURLLabel: "test_url_value_2"}, + }, + }, + result: "test_url_value_2", }, "inspect error": { @@ -312,10 +324,10 @@ func TestRunContainer(t *testing.T) { paramScriptsURL: "http://my.test.url/test?param=one", cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/test -xf - && /opt/test/scripts/%s", api.Assemble)}, }, - "scriptsInsideImage": { + "scriptsInsideImageEnvironment": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{ScriptsURL + "=image:///opt/bin/"}, + Env: []string{ScriptsURLEnvironment + "=image:///opt/bin/"}, }, Config: &docker.Config{}, }, @@ -323,10 +335,21 @@ func TestRunContainer(t *testing.T) { externalScripts: false, cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /tmp -xf - && /opt/bin/%s", api.Assemble)}, }, - "scriptsInsideImageWithParamLocation": { + "scriptsInsideImageLabel": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{ScriptsURL + "=image:///opt/bin"}, + Labels: map[string]string{ScriptsURLLabel: "image:///opt/bin/"}, + }, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: false, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /tmp -xf - && /opt/bin/%s", api.Assemble)}, + }, + "scriptsInsideImageEnvironmentWithParamLocation": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Env: []string{ScriptsURLEnvironment + "=image:///opt/bin"}, }, Config: &docker.Config{}, }, @@ -335,10 +358,33 @@ func TestRunContainer(t *testing.T) { paramLocation: "/opt/sti", cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/sti -xf - && /opt/bin/%s", api.Assemble)}, }, - "paramLocationFromImageVar": { + "scriptsInsideImageLabelWithParamLocation": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{Location + "=/opt", ScriptsURL + "=http://my.test.url/test?param=one"}, + Labels: map[string]string{ScriptsURLLabel: "image:///opt/bin"}, + }, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: false, + paramLocation: "/opt/sti", + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/sti -xf - && /opt/bin/%s", api.Assemble)}, + }, + "paramLocationFromImageEnvironment": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Env: []string{LocationEnvironment + "=/opt", ScriptsURLEnvironment + "=http://my.test.url/test?param=one"}, + }, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: true, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt -xf - && /opt/scripts/%s", api.Assemble)}, + }, + "paramLocationFromImageLabel": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Labels: map[string]string{LocationLabel: "/opt", ScriptsURLLabel: "http://my.test.url/test?param=one"}, }, Config: &docker.Config{}, }, diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index fe6fb454f..930bc1175 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -122,7 +122,7 @@ func NewTarTimeoutError() error { Message: fmt.Sprintf("timeout waiting for tar stream"), Details: nil, ErrorCode: TarTimeoutError, - Suggestion: "check the sti-helper script if it accepts tar stream for assemble and sends for save-artifacts", + Suggestion: "check the Source-To-Image scripts if it accepts tar stream for assemble and sends for save-artifacts", } } @@ -155,7 +155,7 @@ func NewInstallError(script string) error { Message: fmt.Sprintf("failed to install %v", script), Details: nil, ErrorCode: InstallError, - Suggestion: "provide URL with STI scripts with -s flag or check the image if it contains STI_SCRIPTS_URL variable set", + Suggestion: "provide URL with Source-To-Image scripts with -s flag or check the image if it contains io.openshift.sti.scripts-url label set", } } @@ -166,7 +166,7 @@ func NewInstallRequiredError(scripts []string) error { Message: fmt.Sprintf("failed to install %v", scripts), Details: nil, ErrorCode: InstallErrorRequired, - Suggestion: "provide URL with STI scripts with -s flag or check the image if it contains STI_SCRIPTS_URL variable set", + Suggestion: "provide URL with Source-To-Image scripts with -s flag or check the image if it contains io.openshift.sti.scripts-url label set", } } diff --git a/test/integration/images/sti-fake-env/Dockerfile b/test/integration/images/sti-fake-env/Dockerfile new file mode 100644 index 000000000..f20bbcffc --- /dev/null +++ b/test/integration/images/sti-fake-env/Dockerfile @@ -0,0 +1,19 @@ +# +# This is basic fake image used for testing STI. +# +FROM busybox + +RUN mkdir -p /sti-fake/src && \ +# FIXME: The 'default' user has conflicting UID (1000). This should be fixed in +# how source-to-image handles UID's during isolation. +# + deluser default + +WORKDIR / + +# Need to serve the scripts from localhost so any potential changes to the +# scripts are made available for integration testing. +# +# Port 23456 must match the port used in the http server in STI's +# integration_test.go +ENV STI_SCRIPTS_URL http://127.0.0.1:23456/.sti/bin diff --git a/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile b/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile index 4d9259711..1b5a4183d 100644 --- a/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile +++ b/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile @@ -16,4 +16,4 @@ ADD scripts/.sti/bin/run /tmp/scripts/ ADD scripts/.sti/bin/assemble /tmp/scripts/ # Scripts are already in the image and this is their location -ENV STI_SCRIPTS_URL image:///tmp/scripts/ +LABEL io.openshift.sti.scripts-url=image:///tmp/scripts/ diff --git a/test/integration/images/sti-fake-scripts/Dockerfile b/test/integration/images/sti-fake-scripts/Dockerfile index bf5e47c12..803b4e186 100644 --- a/test/integration/images/sti-fake-scripts/Dockerfile +++ b/test/integration/images/sti-fake-scripts/Dockerfile @@ -15,4 +15,4 @@ WORKDIR / ADD scripts/.sti/bin/ /tmp/scripts/ # Scripts are already in the image and this is their location -ENV STI_SCRIPTS_URL image:///tmp/scripts/ +LABEL io.openshift.sti.scripts-url=image:///tmp/scripts/ diff --git a/test/integration/images/sti-fake/Dockerfile b/test/integration/images/sti-fake/Dockerfile index f20bbcffc..be5386d85 100644 --- a/test/integration/images/sti-fake/Dockerfile +++ b/test/integration/images/sti-fake/Dockerfile @@ -16,4 +16,4 @@ WORKDIR / # # Port 23456 must match the port used in the http server in STI's # integration_test.go -ENV STI_SCRIPTS_URL http://127.0.0.1:23456/.sti/bin +LABEL io.openshift.sti.scripts-url=http://127.0.0.1:23456/.sti/bin