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

Inject claim and not run #2697

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Conversation

carolynvs
Copy link
Member

What does this change

We were accidentally injecting Porter's run represetation, which is very similar to the CNAB claim but isn't exactly the same. I've updated the code to add a test to ensure that we don't break injecting the claim.

For context, Porter doesn't use the injected claim but it is an optional part of the CNAB Claims spec, so while unused, if we are going to say we support it we should do it to spec.

What issue does it fix

Closes #2696

Notes for the reviewer

We need to communicate this in the release notes in case any bundle authors were relying on the old bugged behavior.

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

We were accidentally injecting Porter's run represetation, which is _very_ similar to the CNAB claim but isn't exactly the same. I've updated the code to add a test to ensure that we don't break injecting the claim.

For context, Porter doesn't use the injected claim but it is an optional part of the CNAB Claims spec, so while unused, if we are going to say we support it we should do it to spec.

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

@sgettys sgettys 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 eb56c2d into getporter:main Apr 3, 2023
@carolynvs carolynvs deleted the inject-claim branch April 3, 2023 19:34
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.

Porter is injecting run into /cnab/claim.json
2 participants