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

Include sensitive flag in yaml/json output of porter explain for outputs and parameters #2228

Closed
carolynvs opened this issue Jul 6, 2022 · 10 comments
Assignees
Labels
2 - 🍕 Pizza should be eaten daily good first issue Good for new contributors user experience 🌈💖 Make it easier for everyone to use Porter

Comments

@carolynvs
Copy link
Member

This issue started off as a discussion at #2193. When someone is automating retrieving values to run a bundle from porter explain, it is helpful to know if a parameter or output is sensitive. For example, they would know that sensitive values always should be retrieved from a secret store.

I don't want to add more information to the human readable display for the explain command, as we want to limit the info displayed to actionable data and I think this case is really about automation.

So lets add the sensitive flag (boolean) to both the json and yaml parameters and outputs sections in the output of porter explain.

For example, here is a bundle that has sensitive parameters and outputs:

$ porter explain -r ghcr.io/getporter/examples/sensitive-data:v0.1.0 -o yaml
name: examples/sensitive-data
description: An example bundle that generates sensitive data
version: 0.1.0
porterVersion: v1.0.0-alpha.19
parameters:
  - name: password
    type: string
    default: null
    applyTo: All Actions
    description: ""
    required: true
outputs:
  - name: name
    type: string
    applyTo: All Actions
    description: ""
mixins:
  - exec

After the change, the output should include the sensitive field:

$ porter explain -r ghcr.io/getporter/examples/sensitive-data:v0.1.0 -o yaml
name: examples/sensitive-data
description: An example bundle that generates sensitive data
version: 0.1.0
porterVersion: v1.0.0-alpha.19
parameters:
  - name: password
    type: string
    sensitive: true
    default: null
    applyTo: All Actions
    description: ""
    required: true
outputs:
  - name: name
    type: string
    sensitive: true
    applyTo: All Actions
    description: ""
mixins:
  - exec

The change should be made in the following file: https://github.com/getporter/porter/blob/release/v1/pkg/porter/explain.go and the testdata used in the associated test file updated to match the new expected output. You can either update the testdata files by hand, or you can call mage UpdateTestfiles after changing the code, and that will update the testdata files to match the new output.

See our Contributing Tutorial and New Contributor Guide for help getting started contributing to Porter.

@carolynvs carolynvs added good first issue Good for new contributors user experience 🌈💖 Make it easier for everyone to use Porter 2 - 🍕 Pizza should be eaten daily labels Jul 6, 2022
@MadhuMPandurangi
Copy link
Contributor

Hi @carolynvs, I'm interested to contribute to this issue. I request you to assign me the same. I had requested to issue #2324 too. I'm fine with any issue.

@VinozzZ
Copy link
Contributor

VinozzZ commented Sep 8, 2022

Hi @MadhuMPandurangi , thank you for taking on this issue! If you have any questions, feel free to reach out to us!
If you haven't read through our contributing guide, here's the link to it: https://porter.sh/contribute/,

@carolynvs
Copy link
Member Author

@MadhuMPandurangi We encourage contributors to only work on a single issue at a time, so that issues stay "available" for others to pick up until someone is actively working on them. How about you test out Porter with Artifactory first, and then when that's complete, then start on this one.

@MadhuMPandurangi
Copy link
Contributor

Thank you @carolynvs @VinozzZ I'm going through the project and I'm happy to be on your team.

@MadhuMPandurangi
Copy link
Contributor

Built a sample bundle that prints the name is done. I looked at the file https://github.com/getporter/porter/blob/release/v1/pkg/porter/explain.go but I didn't get how to make changes or what to edit in this file. Can anyone please help?

@carolynvs
Copy link
Member Author

Hi @MadhuMPandurangi,

You need to add the sensitive flag to PrintableParameter, in

type PrintableParameter struct {
param *bundle.Parameter
Name string `json:"name" yaml:"name"`
Type interface{} `json:"type" yaml:"type"`
Default interface{} `json:"default" yaml:"default"`
ApplyTo string `json:"applyTo" yaml:"applyTo"`
Description string `json:"description" yaml:"description"`
Required bool `json:"required" yaml:"required"`
}

Then populate that field using the result of bun.IsSensitiveParameter(p) when we create a display representation of the bundle for the explain command:

pp := PrintableParameter{param: &v}
pp.Name = p
pp.Type = bun.GetParameterType(def)
pp.Default = def.Default
pp.ApplyTo = generateApplyToString(v.ApplyTo)
pp.Required = v.Required
pp.Description = v.Description

The yaml/json output of the explain command will automatically pick up your change so you don't need to do anything to get the new sensitive field to print out. Since we are not updating the human readable output of the explain command, there are no associated documentation changes to make this time.

When you run the unit tests with mage testunit, you should see tests fail after your change is completed because the output of the explain command now has the sensitive field included. Run mage updateTestFiles to synchronize the test files with the new output.

@MadhuMPandurangi
Copy link
Contributor

Screenshot from 2022-10-27 08-24-03
Can see the output before build and after the build.
Added sensitive flag and successfully displayed.

@MadhuMPandurangi
Copy link
Contributor

MadhuMPandurangi commented Oct 27, 2022

Do I proceed with PR?
Any document to look at before raising PR for this?

@carolynvs
Copy link
Member Author

Looks good! Yes please create a pull request. We have a template that you will be prompted to fill out when you open it that explains the information we are looking for. 👍

@MadhuMPandurangi
Copy link
Contributor

MadhuMPandurangi commented Oct 28, 2022

Hi @carolynvs @VinozzZ, please review the PR(#2440).

Thank You

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 good first issue Good for new contributors user experience 🌈💖 Make it easier for everyone to use Porter
Projects
None yet
Development

No branches or pull requests

4 participants