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 containeranalysis Note and Occurence for attestation #3564

Merged
merged 3 commits into from
May 27, 2020

Conversation

emilymye
Copy link
Contributor

Release Note Template for Downstream PRs (will be copied)

container_analysis: Added top-level generic note fields to `google_container_analysis_note`
`google_container_analysis_occurence`

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 418 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 3 files changed, 418 insertions(+), 4 deletions(-))
TF Conversion: Diff ( 1 file changed, 90 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

@emilymye emilymye force-pushed the containeranalysis branch from 31d29d8 to 59f5fc3 Compare May 25, 2020 02:08
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 411 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 3 files changed, 411 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 1 file changed, 90 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 12 files changed, 1714 insertions(+), 71 deletions(-))
Terraform Beta: Diff ( 12 files changed, 1758 insertions(+), 71 deletions(-))
TF Conversion: Diff ( 2 files changed, 248 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

1 similar comment
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 12 files changed, 1714 insertions(+), 71 deletions(-))
Terraform Beta: Diff ( 12 files changed, 1758 insertions(+), 71 deletions(-))
TF Conversion: Diff ( 2 files changed, 248 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

@emilymye emilymye requested a review from danawillow May 26, 2020 16:12
name = "<%= ctx[:vars]["attestor"] %>"
attestation_authority_note {
note_reference = google_container_analysis_note.note.name
public_keys {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

things to consider as follow up - this might need to be a set? I didn't test it in-depth in this PR but I'd probably split it out anyways

@@ -51,10 +51,12 @@ func BootstrapKMSKeyWithPurpose(t *testing.T, purpose string) bootstrappedKMS {
* a KMS key.
**/
func BootstrapKMSKeyWithPurposeInLocation(t *testing.T, purpose, locationID string) bootstrappedKMS {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other followup - I also want to rename these methods to have Shared/Default/... in the name so we know the name is the same across them, or add name param

@emilymye emilymye force-pushed the containeranalysis branch from 8ccc9cd to 26e0316 Compare May 26, 2020 16:34
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 11 files changed, 1699 insertions(+), 70 deletions(-))
Terraform Beta: Diff ( 11 files changed, 1743 insertions(+), 70 deletions(-))
TF Conversion: Diff ( 2 files changed, 248 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

@emilymye emilymye requested a review from slevenick May 26, 2020 18:02
Copy link
Contributor

@slevenick slevenick left a comment

Choose a reason for hiding this comment

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

Only real actionable question is around Occurrence project/noteName field

products/containeranalysis/api.yaml Outdated Show resolved Hide resolved
products/containeranalysis/api.yaml Show resolved Hide resolved
products/containeranalysis/api.yaml Outdated Show resolved Hide resolved
products/containeranalysis/api.yaml Show resolved Hide resolved
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 11 files changed, 1699 insertions(+), 70 deletions(-))
Terraform Beta: Diff ( 11 files changed, 1743 insertions(+), 70 deletions(-))
TF Conversion: Diff ( 2 files changed, 248 insertions(+), 1 deletion(-))
TF OiCS: Diff ( 5 files changed, 124 insertions(+), 1 deletion(-))

@emilymye emilymye requested a review from slevenick May 26, 2020 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants