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

Fail HCP datasources for revoked iteration #11854

Merged
merged 3 commits into from
Jun 21, 2022

Conversation

sylviamoss
Copy link
Contributor

@sylviamoss sylviamoss commented Jun 17, 2022

HCP Packer will stop replacing the image id/url with error_revoked so the hcp datasource will fail when querying revoked iterations. This reverts #11624

@sylviamoss sylviamoss requested a review from a team as a code owner June 17, 2022 12:50
@sylviamoss sylviamoss changed the title Dail datasources for revoked iteration Fail HCP datasources for revoked iteration Jun 17, 2022
@@ -96,7 +96,7 @@ require (
github.com/hashicorp/packer-plugin-vmware v1.0.7
github.com/hashicorp/packer-plugin-vsphere v1.0.5
github.com/hashicorp/packer-plugin-yandex v1.1.1
github.com/scaleway/packer-plugin-scaleway v1.0.5
github.com/scaleway/packer-plugin-scaleway v1.0.4
Copy link
Contributor Author

@sylviamoss sylviamoss Jun 17, 2022

Choose a reason for hiding this comment

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

Tag v1.0.5 doesn't exist anymore

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Is this behavior captured in the datasource docs?

Can you update #11849 with a changelog entry as well

@sylviamoss
Copy link
Contributor Author

No, but I should definitely update the documentation. Let me do that and update the changelog!

Comment on lines 131 to 135
if !revokeAt.IsZero() && revokeAt.Before(time.Now().UTC()) {
// If RevokeAt is not a zero date and is before NOW, it means this iteration is revoked and should not be used
// to build new images.
return nil, fmt.Errorf("the iteration %s is revoked and can not be used on Packer builds",
resp.Payload.Iteration.ID)
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized that the error check for revoke_at is in the registry service code and not in the data source code itself. Is there any reason why you went that route as opposed to putting it in the data source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just reverting the first implementation and haven't thought too much about but I think it makes sense to put the check in the data source instead! So I updated the PR.

sylviamoss added a commit that referenced this pull request Jun 21, 2022
sylviamoss added a commit that referenced this pull request Jun 21, 2022
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

🚢

@sylviamoss sylviamoss merged commit b849ace into main Jun 21, 2022
@sylviamoss sylviamoss deleted the hpr-513_revoked_iterations_fail_datasource branch June 21, 2022 15:24
nywilken pushed a commit that referenced this pull request Jun 21, 2022
nywilken added a commit that referenced this pull request Jun 21, 2022
nywilken added a commit that referenced this pull request Jun 21, 2022
@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 Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants