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

Refactor secret source data structures #2729

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

carolynvs
Copy link
Member

@carolynvs carolynvs commented Apr 14, 2023

What does this change

As I was working on advanced dependencies, I kept running into confusion about what some of these structs and fields represented. For example, a source mapping had both a Value field (representing the resolved value) and a Source.Value representing a hint to the secret strategy on how to resolve the value of parameter or credential.

I have renamed the data structures and fields a bit to better align with how they are used.

The renames happened in pkg/secrets/stragegy.go, the rest of the file just propagate the name changes.

What issue does it fix

None

Notes for the reviewer

I can make this a smaller PR by just renaming the top level Value field to ResolvedValue, but I think the other renames may be helpful as well for anyone maintaining the code. Within the same data structure we were using Value to mean 3 different things...

Let me know what you think, we don't have to do any of this unless it makes the code more clear!

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

Reviewer Checklist

  • Comment with /azp run test-porter-release if a magefile or build script was modified
  • Comment with /azp run porter-integration if it's a non-trivial PR

As I was working on advanced dependencies, I kept running into confusion about what some of these structs and fields represented. For example, a source mapping had both a Value field (representing the resolved value) and a Source.Value representing a hint to the secret strategy on how to resolve the value of parameter or credential.

I have renamed the data structures and fields a bit to better align with how they are used.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs marked this pull request as ready for review April 17, 2023 14:22
Copy link
Contributor

@arschles arschles left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@carolynvs carolynvs merged commit c0dcb28 into getporter:main Apr 19, 2023
@carolynvs carolynvs deleted the rename-source-mapping-structs branch April 19, 2023 19:54
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.

3 participants