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

Support skipping parameters and credentials during generate #3100

Merged
merged 5 commits into from
May 13, 2024

Conversation

kichristensen
Copy link
Contributor

@kichristensen kichristensen commented May 7, 2024

What does this change

Adds a skip option to the prompt allow users to skip parameter and credential generation of specific entries.

The prompt will look like this:

Generating new credential test2 from bundle porter-hello

==> 2 credentials required for bundle porter-hello

? How would you like to set credential "kubeconfig"
   [Use arrows to move, enter to select, type to filter]
  secret
  specific value
  environment variable
  file path
  shell command
> skip

What issue does it fix

Closes #3074 and closes #3075

Notes for the reviewer

No tests have been created for this as it requires user interaction

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Adds a `skip` option to the prompt allow users to skip parameter
and credential generation of specific entries.

Signed-off-by: Kim Christensen <[email protected]>
@@ -52,7 +53,7 @@ func genSurvey(name string, surveyType SurveyType) (secrets.SourceMap, error) {
// extra space-suffix to align question and answer. Unfortunately misaligns help text
sourceTypePrompt := &survey.Select{
Message: fmt.Sprintf("How would you like to set %s %q\n ", surveyType, name),
Options: []string{questionSecret, questionValue, questionEnvVar, questionPath, questionCommand},
Options: []string{questionSecret, questionValue, questionEnvVar, questionPath, questionCommand, questionSkip},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you handling if required is set to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go catch, no currently was focusing on the parameters, noticed that is was used for both credentials and parameters, and forgot about. I will update

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schristoff Updated, to only allow skipping parameters or credentials that aren't required. Discover that the CNAB bundle has a required parameter for both credentials and paramters. All parameters without a default value is required in the accroding the CNAB bundle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about changing this to passing in an opts ...Options and those options contain description and required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schristoff that could make sense, it would allow us to easily extend the generator with new options over time. My only "concern" is if it is a refactor that gives value long term. I mean how many options would there be? But after the talk about more metadata on credentials it might actually be worth it.
I will update try to update the PR to use options, then we can see how it would look and feel.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is too much work to implement - nbd on choosing to move forward with this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schristoff I have updated this PR to use options, what do you think?

@kichristensen kichristensen merged commit 7600978 into getporter:main May 13, 2024
15 checks passed
@kichristensen kichristensen deleted the allowSkipDuringGenerate branch May 13, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants