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

Suggestion: Set custom values during porter build #1789

Closed
carolynvs opened this issue Oct 1, 2021 · 15 comments
Closed

Suggestion: Set custom values during porter build #1789

carolynvs opened this issue Oct 1, 2021 · 15 comments
Assignees
Labels
2 - 🍕 Pizza should be eaten daily gap We missed a spot user experience 🌈💖 Make it easier for everyone to use Porter

Comments

@carolynvs
Copy link
Member

I would like to be able to use the custom section as variables essentially. Here's one thing that I often want to do: tie a bundle directly to a version of an application. It's really hard to do without scripting out editing the dockerfile and porter.yaml so injecting like this where porter supports such a common case explicitly would make it much easier to package an app automatically when new versions are released.

$ porter build --custom version=0.2.0
# porter.yaml
custom:
  version: "placeholder"

install:
  - helm3:
      chart: ./charts/myapp
      version: "{{ bundle.custom.version }}"
# Dockerfile

# Install the desired version of the chart
ARG CUSTOM_VERSION=TODO
RUN curl -O https://example.com/charts/$CUSTOM_VERSION/myapp.tgz
RUN tar -xzf myapp.tgz /cnab/app/charts/ && rm myapp.tgz

This change should be made on the release/v1 branch.

@carolynvs carolynvs added user experience 🌈💖 Make it easier for everyone to use Porter gap We missed a spot 2 - 🍕 Pizza should be eaten daily labels Oct 1, 2021
@carolynvs carolynvs added this to Inbox in Porter and Mixins via automation Oct 1, 2021
@joshuabezaleel
Copy link
Contributor

Hi @carolynvs , I want to ask some clarification questions if I may:

  1. What is the difference between the build command attached on the root which is located here (https://github.com/getporter/porter/blob/release/v1/cmd/exec/build.go) and attached on the bundle command here (https://github.com/getporter/porter/blob/release/v1/cmd/porter/bundle.go#L49)? I tried to run porter build and it ran the latter one (the one attached to bundle). Is the former not used anymore?
  2. Will it enable to have more than 1 pair of key-value for the --custom flag? e.g. version=0.2.0 and key2=value so we will have {{ bundle.custom.version }} and {{ bundle.custom.key2 }}

@carolynvs
Copy link
Member Author

We don't explain this explicitly in our CLI documentation but some of our commands are so common, like porter bundle build that we provide an alias for it that is shorter: porter build.

If you run porter help the help output calls out which commands are aliases. The underlying implementation in porter is the same. We group them this way so that if you are exploring our available resources/commands, you can see related commands together (like porter installation upgrade and porter installation uninstall).

As for the --custom flag, it should be bound to a slice so that people can specify it multiple times. If you look at how we define the --param flag for install, that's a good example of how to do that.

@joshuabezaleel
Copy link
Contributor

@carolynvs Ah yes, I forgot about aliases, that explains 😅

$ porter build --custom version=0.2.0
# porter.yaml
custom:
  version: "placeholder"

install:
  - helm3:
      chart: ./charts/myapp
      version: "{{ bundle.custom.version }}"

By the way I'm sorry if I haven't fully got it yet, from the example that you mentioned above does this mean that the 0.2.0 from the command will replace the placeholder value for version on the yaml and thus the value "piped" to the {{bundle.custom.version}}? Since the current flow that I know is that the value for {{bundle.custom.version}} variable on the yaml will be retrieved from custom -> version value on the same yaml file, right?

So does this mean that it will be like below for the particular command?

$ porter build --custom version=0.2.0
# porter.yaml
custom:
  version: "0.2.0"

install:
  - helm3:
      chart: ./charts/myapp
      version: "{{ bundle.custom.version }}"

@carolynvs
Copy link
Member Author

@joshuabezaleel Yup! The example you have there is what we want. Any custom values should be applied to the porter.yaml (there is already an existing bit of code that handles editing the porter.yaml before the build and saving it to .cnab/app/porter.yaml).

Then at runtime, the existing templating mechanism handles injecting the value "0.2.0" when actions from the manifest are executed (like the helm install step).

@joshuabezaleel
Copy link
Contributor

@carolynvs May I ask to be assigned for this one? Will put up a PR tomorrow 🙌

@carolynvs
Copy link
Member Author

Assigned!

I noticed in your example above that you didn't mention the Dockerfile. I wanted to call out that in addition to injecting the custom variable into the porter.yaml, the value is also injected into the docker image when it is built using a build argument.

In the original example I posted, we have a custom variable named "version". So when the invocation image for the bundle is built we should pass a build argument named "CUSTOM_VERSION".

One scenario to consider is how to inject a nested custom variable.

porter.yaml

custom:
  myapp:
    version: "placeholder"

command executed

porter build --custom myapp.version="0.2.2"

final porter.yaml

custom:
  myapp:
    version: "0.2.2"

Dockerfile

ARG CUSTOM_MYAPP_VERSION

Note that the dot . notation for specifying the nested variable needs to be converted to underscores, and then uppercased when setting the build argument. The custom variable myapp.version is turned into a build argument named CUSTOM_MYAPP_VERSION.

Another note about types. Someone should be able to set a boolean or numeric value in addition to a string (let's ignore setting an entire object value for now). So porter build --custom active=true should make the following

custom:
  active: true # true is a boolean, not a quoted string

Same for numbers, porter build --custom number=5 would make

custom:
  number: 5 # 5 is a number  not a quoted string

@joshuabezaleel
Copy link
Contributor

@carolynvs I almost forgot about the Dockerfile, thank you for the kind reminder, Carolyn! I am sorry in advance if it's going to take a little longer since I forgot about this one 😅

Where should the custom input arguments be located inside of the Dockerfile? Let's say I'm using the porter-whalesay bundle with the build command of porter build --custom app.key="value" --custom key2=true, going with my initial assumption, will it be okay to put all of the custom input arguments just below the BUNDLE_DIR? So I guess the Dockerfile produced should be around this one based on the command that I mentioned earlier.

FROM debian:stretch-slim

ARG BUNDLE_DIR
ARG CUSTOM_APP_KEY=value
ARG CUSTOM_KEY2=true

RUN apt-get update && apt-get install -y ca-certificates

ARG DOCKER_VERSION=20.10.7
RUN apt-get update && apt-get install -y curl && \
	curl -o docker.tgz https://download.docker.com/linux/static/stable/x86_64/docker-${DOCKER_VERSION}.tgz && \
    tar -xvf docker.tgz && \
    mv docker/docker /usr/bin/docker && \
    chmod +x /usr/bin/docker && \
    rm docker.tgz


COPY . $BUNDLE_DIR
RUN rm $BUNDLE_DIR/porter.yaml
RUN rm -fr $BUNDLE_DIR/.cnab
COPY .cnab /cnab
WORKDIR $BUNDLE_DIR
CMD ["/cnab/app/run"]

@carolynvs
Copy link
Member Author

You don't need to change the Dockerfile at all! 🎉 The bundle author will declare and use ARG statements if they need it. Porter should just always be passing them in so that they are available if the author wants them. We use the docker library, so it's not exactly the same but essentially when we build the invocation image we should always be passing in any custom values with --build-arg CUSTOM_VARIABLE=VALUE --build-arg CUSTOM_VARIABLE2=VALUE2. Docker will ignore them if they aren't used by the Dockerfile.

@joshuabezaleel
Copy link
Contributor

@carolynvs Ah yes, what I meant by changing the Dockerfile was actually by passing the custom values as the BuildArgs when the function BuildInvocationImage gets called. I found that this function was implemented on 2 packages, one is pkg/build/docker/docker.go and another one is pkg/build/buildkit/buildx.go.
Should we also pass these custom values on the BuildArgs for the BuildInvocationImage function on the buildkit/buildx.go or only at the docker.docker.go?

@joshuabezaleel
Copy link
Contributor

joshuabezaleel commented Jan 18, 2022

@carolynvs Another thing that I want to ask for help and need to consult is regarding the variable type handling. I stumbled upon these and it's been days since I haven't found any good workaround 🤔 .

I noticed that the spf13/pflag library that is used strips all the double-quotes from the values that are supplied to the flags when they were parsed. So porter build --custom version="0.2.0" will have the version value of 0.2.0 which will work the same if we don't use double quote for the value (porter build --custom version=0.2.0). This yield to some questions and issues as follows:

  1. If user tried to input the values for the custom flags, when they want to use double-quote to differentiate between what supposed to be string inputs and not using double-quote for boolean and number inputs, it will yield the same because the double-quote will be stripped anyways.
  2. For checking whether the value was of boolean (true/false) or number type, I use this kind of checking which I am afraid will be considered too "dirty" and you might have some other recommendations.
  3. Also to put the double-quote in the beginning and the ending of what supposed to be the string type value, I tried to use escaped double-quote v = fmt.Sprintf("\"%s\"", v) but what was get printed to the .yaml has extra single-quote at the beginning and ending of the value (version: '"stringvalue"' instead of the intended version: "stringvalue").
    image

Along with the comment I attached the recording of the prototype that shows what I described on the 3 points earlier. Please pardon all of the logs that I used to debug 😅

porter-1789-custom-values-porter-build.1.mp4

Truly sorry for the trouble and thank you so much in advance, Carolyn! 🙂

@carolynvs
Copy link
Member Author

Should we also pass these custom values on the BuildArgs for the BuildInvocationImage function on the buildkit/buildx.go or only at the docker.docker.go?

Porter supports two different build drivers: standard docker and the new docker buildkit. So you will need to add support for passing build arguments in both implementations.

@carolynvs
Copy link
Member Author

As for the type stuff, I think my use of quotes was confusing. Let's step back and think about what will happen in each case for various types and how we can avoid caring what type they specified for a variable.

string

# the command --custom myvar=something yields the following porter.yaml
# myvar will be treated like a string by porter when it reads the yaml file at runtime.
custom:
  myvar=something

boolean

# the command --custom myvar=true yields the following porter.yaml
# myvar will be treated like a boolean by porter when it reads the yaml file at runtime.
custom:
  myvar=true

number

# the command --custom myvar=1 yields the following porter.yaml
# myvar will be treated like a number by porter when it reads the yaml file at runtime.
custom:
  myvar=1

Let's think for a sec about if it matters how the type is interpreted in the yaml anyway. The custom variable is always consumed by the bundle as a string:

  1. In the Dockerfile, --build-arg / ARG is always a string anyway. If we knew it was an int, it wouldn't change how we passed it into docker.
  2. In porter's templating, {{ bundle.custom.myvar }} is also treated as a string when we render a template for any mixin steps that use that variable. The type doesn't change how it's substituted into the file.

If I'm wrong and it does matter, I think quotes can be forced by using tricks like '"1"'. I haven't tried this out but I think it would look like this (I haven't seen your implementation yet but I believe its possible to get this to work).

number that I want to be a string

# the command --custom myvar='"1"' yields the following porter.yaml
# myvar will be treated like a number by porter when it reads the yaml file at runtime.
custom:
  myvar='"1"'

Based on that, does this help answer your question and get rid of any type checking you had put in place? Or is there still more to figure out for that part?

@joshuabezaleel
Copy link
Contributor

@carolynvs What I am thinking about was when we want to pipe the boolean/integer custom values (1) to properties that only accept boolean/integer values or (2) using it as the boolean/integer custom values, not string. The example that I am using is with docker as follows, using the whalesay bundle.

# porter build --custom booleanCustomInput=true

# porter.yaml
custom:
   booleanCustomInput: true

say:
   - docker:
        run:
            image: "docker/whalesay:latest"
            rm: {{ bundle.custom.booleanCustomInput }}

If I want to use the bundle.custom.booleanCustomInput value which should have boolean value of true as the value for the rm property, using it with the variable of rm: {{ bundle.custom.booleanCustomInput}} (without the double-quote like the "{{bundle.custom.version}}" mentioned in at the first example above) will yield an error at the porter build process with the message of

Error: error unmarshaling the untyped manifest: yaml: invalid map key: map[string]interface {}{"bundle.custom.customboolean":""}

While if we use double-quote, the porter build will succeed but it will fails at the porter invoke process since the rm property expects boolean value (true), not string ("true"), which we will get if we encapsulate it with double-quote around it ("{{ bundle.custom.booleanCustominput }}"). The porter.yaml file and the error message is as follows.

# porter build --custom booleanCustomInput=true

# porter.yaml
custom:
   booleanCustomInput: true

say:
   - docker:
        run:
            image: "docker/whalesay:latest"
            rm: "{{ bundle.custom.booleanCustomInput }}"
# porter invoke whalesay --action=say

invoking whalesay...
Using runtime driver docker
executing say action from whalesay (installation: whalesay)
No existing bundle state to unpack
err: could unmarshal input:
 say:
  - docker:
      run:
        arguments:
          - cowsay
          - "false"
        image: docker/whalesay:latest
        rm: "false"
: yaml: unmarshal errors:
  line 5: cannot unmarshal !!str `false` into bool
Error: could unmarshal input:
 say:
  - docker:
      run:
        arguments:
          - cowsay
          - "false"
        image: docker/whalesay:latest
        rm: "false"
: yaml: unmarshal errors:
  line 5: cannot unmarshal !!str `false` into bool
Packing bundle state...
Error: 1 error occurred:
        * mixin execution failed: exit status 1


Error: 1 error occurred:
        * container exit code: 1, message: <nil>

@carolynvs
Copy link
Member Author

This error isn't because of a boolean. It's because the template syntax wasn't quoted (its not valid yaml).

Error: error unmarshaling the untyped manifest: yaml: invalid map key: map[string]interface {}{"bundle.custom.customboolean":""}

say:
   - docker:
        run:
            image: "docker/whalesay:latest"
            rm: {{ bundle.custom.booleanCustomInput }}

should be

say:
   - docker:
        run:
            image: "docker/whalesay:latest"
            rm: "{{ bundle.custom.booleanCustomInput }}"

I really don't want you to go down a rabbit hole on the problems raised in #316. I am working on a fix for that by switching our template delimiters to work better with yaml but in the meantime don't try to solve "type problems" in this issue. If it ends up only working for string that's fine for now.

@github-actions
Copy link

Closed by #1900.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - 🍕 Pizza should be eaten daily gap We missed a spot user experience 🌈💖 Make it easier for everyone to use Porter
Projects
Development

No branches or pull requests

2 participants