zarf icon indicating copy to clipboard operation
zarf copied to clipboard

675 load package variables from file

Open Racer159 opened this issue 2 years ago • 10 comments

Description

This introduces the ability to load package variables from files and refines the workflow to reduce confusion.

Related Issue

Fixes #675

Type of change

  • [X] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist before merging

  • [X] Tests have been added/updated as necessary (add the needs-tests label)
  • [X] Documentation has been updated as necessary (add the needs-docs label)
  • [X] An ADR has been written as necessary (add the needs-adr label) [ 1 2 3 ]
  • [X] (Optional) Changes have been linted locally with golangci-lint. (NOTE: We haven't turned on lint checks in the pipeline yet so linting may be hard if it shows a lot of lint errors in places that weren't touched by changes. Thus, linting is optional right now.)

Racer159 avatar Aug 24 '22 15:08 Racer159

@Madeline-UX from talking with @jeff-mccoy last night I think this PR has a need to validate with users (potentially just from the k8s slack) to gather input on what they expect. With @jeff-mccoy, @RothAndrew and @YrrepNoj being devs there can be a bias with the different ways that instantiating a "variable" can be done with different languages acting differently (go and bash will initialize a string as an empty string, javascript, java, c, and others will initialize as null or undefined, and python will force you to set it to None or an empty string). I think those three behaviors would be a good baseline to determine expectations.

I think this could simply be using the set of scenarios outlined in the ADR and asking users what they think would happen in those scenarios.

Racer159 avatar Sep 08 '22 16:09 Racer159

Specifically:

variables:
  - name: "NOTHING"
  - name: "NO_PROMPT"
    default: "no-prompt"
  - name: "NO_DEFAULT"
    prompt: true
  - name: "BOTH"
    default: "both"
    prompt: true
zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_PROMPT=world --set NO_DEFAULT=today --set BOTH=tomorrow

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_DEFAULT=today

zarf package deploy package.zst --confirm --set NOTHING=hello

zarf package deploy package.zst --confirm

zarf package deploy package.zst

Racer159 avatar Sep 08 '22 17:09 Racer159

@Racer159 - I agree that more validation is needed. I am working off of the assumption that we can use the UI to get feedback from users from different skill levels. I am working to integrate deploy time variables into a prototyped UI and have already been getting feedback on the feature (mostly questions at this time.) I would like some help with creating a more realistic prototype @RothAndrew that shows all the different options we are proposing here and see if that makes sense to users.

Madeline-UX avatar Sep 08 '22 17:09 Madeline-UX

Specifically:

variables:
  - name: "NOTHING"
  - name: "NO_PROMPT"
    default: "no-prompt"
  - name: "NO_DEFAULT"
    prompt: true
  - name: "BOTH"
    default: "both"
    prompt: true
zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_PROMPT=world --set NO_DEFAULT=today --set BOTH=tomorrow

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_DEFAULT=today

zarf package deploy package.zst --confirm --set NOTHING=hello

zarf package deploy package.zst --confirm

zarf package deploy package.zst

@Racer159 That looks like what the package definition would be - but what would the user see on deployment in the CLI?

Madeline-UX avatar Sep 08 '22 17:09 Madeline-UX

Possible questions we could ask:

  1. If you create a variable called "FOO" that does not have a default, and is not set to prompt the user for a value, what should happen?

    1. The system will automatically assign "" (an empty value / empty string) as the value of FOO and continue processing
    2. The system will stop processing and return an error asking the user to set a value for FOO using <command> --set "FOO=YourValueHere"
    3. Something else?
  2. If you create a variable called "BAR" that has a default and is not set to prompt the user for a value, what should happen?

    1. It uses the default unless the user has used <command> --set "BAR=YourValueHere". If --set was used it uses that as the value instead.
    2. Something else?
  3. If you create a variable called "BAZ" that does not have a default and is set to prompt the user for a value, what should happen if the user uses --confirm, which overrides interactive prompts?

    1. The system will automatically assign "" (an empty value / empty string) as the value of BAZ and continue processing
    2. The system will stop processing and return an error asking the user to set a value for BAZ using <command> --set "BAZ=YourValueHere"
    3. Something else?
  4. If you create a variable called "QUX" that has a default and is set to prompt the user for a value, what should happen if the user uses --confirm, which overrides interactive prompts?

    1. ?

RothAndrew avatar Sep 08 '22 20:09 RothAndrew

@RothAndrew This is helpful. I think to get the feedback we are looking for having common or easily understood examples will be helpful. The users I have discussed variables with so far do not understand what they are being asked at a general level but the context of (DNS, Ports, etc) helps. So is there a real/common example for each of those use cases outlined above?

Madeline-UX avatar Sep 08 '22 22:09 Madeline-UX

@RothAndrew @Racer159 @jeff-mccoy Here are some of the mock up's I did for variables in the UI. Don't think I captured all the different ways variables work but trying to explore how we would present all the different types of variables to a user. Desired outcome is that the user completes all variables correctly and is able to successfully deploy as they intended without having to edit or redo the deployment due to a variable being incorrectly implemented.

Screen Shot 2022-09-08 at 6 02 16 PM Screen Shot 2022-09-08 at 6 02 02 PM Screen Shot 2022-09-08 at 6 02 07 PM

Madeline-UX avatar Sep 08 '22 23:09 Madeline-UX

The top is the package definition, and then the bottom are the commands the user runs.

Racer159 avatar Sep 09 '22 18:09 Racer159

The top is the package definition, and then the bottom are the commands the user runs.

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_PROMPT=world --set NO_DEFAULT=today --set BOTH=tomorrow

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_DEFAULT=today

zarf package deploy package.zst --confirm --set NOTHING=hello

zarf package deploy package.zst --confirm

zarf package deploy package.zst

  • So I don't see how this invites user interaction. - It also does not follow the current Zarf model to my knowledge for promoting user input.

Madeline-UX avatar Sep 09 '22 19:09 Madeline-UX

The top is the package definition, and then the bottom are the commands the user runs.

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_PROMPT=world --set NO_DEFAULT=today --set BOTH=tomorrow

zarf package deploy package.zst --confirm --set NOTHING=hello --set NO_DEFAULT=today

zarf package deploy package.zst --confirm --set NOTHING=hello

zarf package deploy package.zst --confirm

zarf package deploy package.zst

* So I don't see how this invites user interaction. - It also does not follow the current Zarf model to my knowledge for promoting user input.

The --confirm flag will just execute without needing any interaction, the last command example without a --confirm flag would currently prompt for the following (... is for truncated output):

...
Deploy this Zarf package? (y/N)
...
Please provide a value for 'NO_DEFAULT': 
Please provide a value for 'BOTH' (both):
...

Racer159 avatar Sep 09 '22 21:09 Racer159

Can I get a quick SWAG on when this might merge? DI2-ME updated to the latest version of Zarf but this is what we really need to start the transition to making it so users don't have to fork the repo to use the software factory package

RothAndrew avatar Sep 23 '22 22:09 RothAndrew

I'll poke around this in the next couple of days but think we might go in a slightly different direction with viper.

jeff-mccoy avatar Sep 23 '22 22:09 jeff-mccoy

Cool, yeah I don't really care how it's done, just want the capability.

Are we talking a week? a month? longer? Trying to figure out when to anticipate when the big push / refactor for DI2-ME might be able to be undertaken.

RothAndrew avatar Sep 23 '22 22:09 RothAndrew

@jeff-mccoy @YrrepNoj @RothAndrew @Madeline-UX reset this PR now that the Viper PR has been merged.

Racer159 avatar Oct 05 '22 16:10 Racer159

Still want to discuss this further before moving forward, @Madeline-UX had some useful chats on it last week. Still bottom line: we lack user validation on this that I think we will need first.

jeff-mccoy avatar Oct 05 '22 19:10 jeff-mccoy

@jeff-mccoy @Racer159 I am working to find users to validate this. I have a few who I think will be a good fit.

Madeline-UX avatar Oct 05 '22 20:10 Madeline-UX

I'd like to be one of the users pls

RothAndrew avatar Oct 05 '22 20:10 RothAndrew

Closing this ancient wonder as stale until we can circle back to it at some future point.

jeff-mccoy avatar Feb 01 '23 03:02 jeff-mccoy