fix: stacker convert should include ENV vars into runtime config.

[jmblatten]

Dockerfile ENV defines variables for both build and runtime. Stacker convert, converts ENV for build, but does not include runtime. Stacker uses "environment:" in yaml to define variables for runtime. So, include the ENV variables there so they will show up in the config when running the container.
See https://docs.docker.com/reference/dockerfile/#env

What type of PR is this?
bug

Which issue does this PR fix:
project-stacker#757

What does this PR do / Why do we need it:
See above.

If an issue # is not available please add repro steps and logs showing the issue:
project-stacker#757

Steps:

1. Dockerfile contents:

FROM alpine AS mybase
ENV HELLO=hello!
ENV GOODBYE=goodbye!
RUN echo $HELLO && \
    echo $GOODBYE

2. Converted stacker.yaml

mybase:
  build_env:
    arch: amd64
  from:
    type: docker
    url: docker://alpine
  run:
  - export HELLO=hello!
  - export GOODBYE=goodbye!
  - sh -e -c 'echo $HELLO &&     echo $GOODBYE'

3. Using skopeo inspect

"Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
    ]

Note: env does not include HELLO and GOODBYE in image.

Testing done on this change:

• CI
• Manual:

1. Dockerfile contents:

FROM alpine AS mybase
ENV HELLO=hello!
ENV GOODBYE=goodbye!
RUN echo $HELLO && \
    echo $GOODBYE

2. Converted stacker.yaml contents

mybase:
  build_env:
    arch: amd64
  environment:
    GOODBYE: goodbye!
    HELLO: hello!
  from:
    type: docker
    url: docker://alpine
  run:
  - export HELLO=hello!
  - export GOODBYE=goodbye!
  - sh -e -c 'echo $HELLO &&     echo $GOODBYE'

3. Using skopeo inspect,

"Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
        "GOODBYE=goodbye!",
        "HELLO=hello!"
    ]

Automation added to e2e:
None

Will this break upgrades or downgrades?
No

Does this PR introduce any user-facing change?:
Yes, but only for converted Dockerfiles. The resulting stacker.yaml file will now include an environment section if there were any ENVs in the Dockerfile. This will cause the image runtime to include the environment variables.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[jsatterfield-geico]

Wouldn't lines 179-186 handle this case? I wouldn't think it's possible for layer.Environment to be nil at this point.

[jmblatten]

I agree.
I imitated the lines above it, project-stacker#218-220. It too is checked first above, yet checked again here. I thought I was missing some understanding like scope or whatever. But I just recompiled this fix without lines, 218-224, and nothing broke and I got same results. So changing....

[jsatterfield-geico]

Non-blocking approve. There's nothing wrong with re-checking that layer.Environment is not null, but I don't think it's ever executed and effectively dead code. If you can confirm, you can drop from the patch and I'll re-approve.

[jsatterfield-geico]

LGTM!