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.

Sign up for GitHub

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

Jump to bottom

Fail for revoked iterations #11492

Merged
merged 3 commits into from
Jan 13, 2022
Merged

Fail for revoked iterations #11492

merged 3 commits into from
Jan 13, 2022

Conversation

sylviamoss
Copy link
Member

@sylviamoss sylviamoss commented Jan 12, 2022 •
edited
Loading

This simply adds a check to fail when an iteration is revoked and no longer appropriate to use.

Output is like

Error: Datasource.Execute failed: error retrieving iteration from HCP Packer registry: the iteration associated with the channel dev is revoked and can not be used on Packer builds

  on test.pkr.hcl line 10:
  (source code not available)

@sylviamoss sylviamoss requested a review from a team as a code owner January 12, 2022 16:46
@sylviamoss sylviamoss changed the title Fail for revoked iterations [draft] Fail for revoked iterations Jan 12, 2022
@sylviamoss sylviamoss changed the title [draft] Fail for revoked iterations Fail for revoked iterations Jan 12, 2022
JenGoldstrich
JenGoldstrich approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@JenGoldstrich JenGoldstrich left a comment

Choose a reason for hiding this comment

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

This looks good to me, I have some testing thoughts I'll post in slack

nywilken
nywilken reviewed Jan 12, 2022
View reviewed changes
internal/registry/service.go
opts ...GetIterationOption,
) (*packer_service.PackerServiceGetIterationOK, error) {

func (client *Client) GetIteration(ctx context.Context, bucketSlug string, opts ...GetIterationOption) (*models.HashicorpCloudPackerIteration, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I see that this now returns an iteration instead of the payload. I would check that all callers of this function have been updated to handle the new return value.

Copy link
Member

Choose a reason for hiding this comment

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

disregard this comment - it was a comment to myself as I was reviewing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing this, I wondered for a moment if the two methods should exist, but I couldn't find a reason why, so LGTM !

Copy link
Member Author

Choose a reason for hiding this comment

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

@azr one gets a channel and the other gets the iteration. The get channel one is used in the iteration datasource and the other in the image datasource, and is also used before creating the iteration to check if it already exists!

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, nice !

azr
azr approved these changes Jan 13, 2022
View reviewed changes
Copy link
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

LGTM !

@sylviamoss sylviamoss merged commit 9ffd7ce into master Jan 13, 2022
@sylviamoss sylviamoss deleted the hcp_check_revoked_iteration branch January 13, 2022 11:28
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@azr azr azr approved these changes

@nywilken nywilken nywilken left review comments

@JenGoldstrich JenGoldstrich JenGoldstrich approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sylviamoss @azr @nywilken @JenGoldstrich