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

Call host secrets plugin directly when resolving secrets #3155

Merged
merged 7 commits into from
Aug 23, 2024

Conversation

kichristensen
Copy link
Contributor

@kichristensen kichristensen commented Jun 11, 2024

What does this change

We should not require all secret plugins to call the host secrets plugin for non secret values. Instead we should call the host secrets plugin directly.

What issue does it fix

Closes #2223

Notes for the reviewer

No integration test was added, as there already is an existing integration test covering the case

func TestInstall_fileParam(t *testing.T) {

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! 🙇‍♀️

We should not require all secret plugins to call the host secrets plugin
for non secret values. Instead we should call the host secrets plugin
directly.

Signed-off-by: Kim Christensen <[email protected]>
@kichristensen kichristensen enabled auto-merge (squash) August 17, 2024 20:03
},
}),
}
err := r.loadCredentials(context.Background(), b, &run)
Copy link
Member

Choose a reason for hiding this comment

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

Think we could make a testcontext and pass that in here, but we don't have to (the func is like NewPorterTestContext or NewTestContext ?)

@@ -23,3 +24,7 @@ func NewTestStore(tc *config.TestConfig) TestStore {
func (s TestStore) Close() error {
return s.testPlugin.Close()
}

Copy link
Member

Choose a reason for hiding this comment

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

We should comment on what this logic is reasoning about

Copy link
Member

@schristoff schristoff left a comment

Choose a reason for hiding this comment

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

two minor nits otherwise g2g

@kichristensen kichristensen merged commit d61237f into getporter:main Aug 23, 2024
38 checks passed
@kichristensen kichristensen deleted the callHostSecretsPlugin branch August 25, 2024 20:07
jmcudd pushed a commit to jmcudd/porter that referenced this pull request Sep 21, 2024
)

We should not require all secret plugins to call the host secrets plugin
for non secret values. Instead we should call the host secrets plugin
directly.

Signed-off-by: Kim Christensen <[email protected]>
Co-authored-by: schristoff <[email protected]>
Signed-off-by: John Cudd <[email protected]>
kichristensen added a commit that referenced this pull request Sep 24, 2024
* Simplified the bundle script

Signed-off-by: John Cudd <[email protected]>

* Added contributor

Signed-off-by: John Cudd <[email protected]>

* Updated docs

Signed-off-by: John Cudd <[email protected]>

* Removed trailing space

Signed-off-by: John Cudd <[email protected]>

* Added a note about mixins not being included

Signed-off-by: John Cudd <[email protected]>

* Tweaked the docs a bit

Signed-off-by: John Cudd <[email protected]>

* Added link at top

Signed-off-by: John Cudd <[email protected]>

* Add bundle script to release and netify redirects

Signed-off-by: John Cudd <[email protected]>

* Call host secrets plugin directly when resolving secrets (#3155)

We should not require all secret plugins to call the host secrets plugin
for non secret values. Instead we should call the host secrets plugin
directly.

Signed-off-by: Kim Christensen <[email protected]>
Co-authored-by: schristoff <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Add doc for AKS integration (#3206)

* add doc for AKS integration

Signed-off-by: dejanualex <[email protected]>

* remove merge action and outputs

Signed-off-by: dejanualex <[email protected]>

---------

Signed-off-by: dejanualex <[email protected]>
Co-authored-by: Kim Christensen <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Do not reference CNAB invocation images in documentation (#3200)

* Do not reference CNAB invocation images

As CNAB is an implementation detail as this point, we no longer want
to use the term 'invocation image'. Instead we should use the term
'bundle image'.
This PR ensures that the current references to 'invocation image' is
changed to 'bundle image'.

Signed-off-by: Kim Christensen <[email protected]>

---------

Signed-off-by: Kim Christensen <[email protected]>
Co-authored-by: schristoff <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Bump actions/download-artifact from 3.0.1 to 4.1.7 in /.github/workflows in the github_actions group across 1 directory (#3217)

Bump actions/download-artifact

Bumps the github_actions group with 1 update in the /.github/workflows directory: [actions/download-artifact](https://github.com/actions/download-artifact).

Updates `actions/download-artifact` from 3.0.1 to 4.1.7
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v3.0.1...v4.1.7)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-type: direct:production
  dependency-group: github_actions
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: John Cudd <[email protected]>

* fix: Update version of upload-artifact (#3219)

fix: Update upload-artifact action version

Signed-off-by: Kim Christensen <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Correct the Cosign links that have stopped working (#3220)

fix: Correct the Cosign links that have stopped working

Signed-off-by: Kim Christensen <[email protected]>
Co-authored-by: schristoff <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Upgrade should not be allowed if installation is not installed (#3213)

fix: Upgrade should not be allowed if installation is not installed

Signed-off-by: Kim Christensen <[email protected]>
Signed-off-by: John Cudd <[email protected]>

* Updated documentation to explain mongo

Also removed echo from the extraction instructions.

Signed-off-by: John Cudd <[email protected]>

---------

Signed-off-by: John Cudd <[email protected]>
Signed-off-by: Kim Christensen <[email protected]>
Signed-off-by: dejanualex <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kim Christensen <[email protected]>
Co-authored-by: schristoff <[email protected]>
Co-authored-by: dejanualex <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically call the host secrets plugin
2 participants