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

Add check for revoked iterations to HCP Packer datasources #240

Merged
merged 5 commits into from
Jan 26, 2022
Merged

Add check for revoked iterations to HCP Packer datasources #240

merged 5 commits into from
Jan 26, 2022

Conversation

sylviamoss
Copy link
Contributor

@sylviamoss sylviamoss commented Jan 13, 2022 •
edited
Loading

🛠️ Description

This adds a check for revoked iterations. For HCP Packer GA, revoking iterations will be possible and if an iteration is revoked it must never be used and the terraform plan|apply should fail.

Since I updated the hcp-sdk-go version, I had to make some changes to vault's import and tests. Changes were checked and validated with @kkredit

🚢 Release Note

Release note for CHANGELOG:

* datasource/hcp_packer_image: Add check for revoked iterations [GH-240]
* datasource/hcp_packer_iteration: Add check for revoked iterations [GH-240]
* datasource/hcp_packer_image_iteration: Add check for revoked iterations [GH-240]

🏗️ Acceptance tests

  • Are there any feature flags that are required to use this functionality?
  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAcc_dataSourcePacker'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/... -v -run=TestAcc_dataSourcePackerImage -timeout 120m
?       github.com/hashicorp/terraform-provider-hcp/internal/clients    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/consul     (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      (cached) [no tests to run]
=== RUN   TestAcc_dataSourcePackerImage
--- PASS: TestAcc_dataSourcePackerImage (27.25s)
=== RUN   TestAcc_dataSourcePackerImage_revokedIteration
--- PASS: TestAcc_dataSourcePackerImage_revokedIteration (13.53s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider   41.102s
...

@sylviamoss sylviamoss changed the title check for revoked iterations A check for revoked iterations to HCP Packer datasources Jan 24, 2022
@sylviamoss sylviamoss marked this pull request as ready for review January 24, 2022 17:46
@sylviamoss sylviamoss requested a review from a team January 24, 2022 17:46
@kkredit
Copy link
Contributor

kkredit commented Jan 24, 2022

The Vault-related changes look great. Thanks Moss!

bcmdarroch
bcmdarroch reviewed Jan 24, 2022
View reviewed changes
Copy link
Contributor

@bcmdarroch bcmdarroch left a comment

Choose a reason for hiding this comment

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

Had a couple questions!

go.mod Outdated
@@ -11,7 +11,7 @@ require (
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-version v1.3.0
github.com/hashicorp/hcl/v2 v2.8.2 // indirect
github.com/hashicorp/hcp-sdk-go v0.14.0
github.com/hashicorp/hcp-sdk-go v0.15.1-0.20220112153249-f565607d7cc4
Copy link
Contributor

Choose a reason for hiding this comment

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

I need to get some network updates into the go SDK, so I'll try to release 0.16.0 with your changes tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! So I'll wait for the release to update this one with 0.16.0

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/hashicorp/hcp-sdk-go/releases/tag/v0.16.0 ✨

internal/provider/data_source_packer_iteration.go Outdated
if !time.Time(iteration.RevokeAt).IsZero() {
// If RevokeAt is not a zero date, it means this iteration is revoked and should not be used
// to build new images.
return diag.Errorf("the iteration %s is revoked and can not be used", 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.

Can we advise the user what to do/change when they run into this error?

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 think we can, I will try to think of a better text!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! I also improved some of the tests

internal/clients/client.go
@@ -17,8 +17,8 @@ import (
cloud_consul "github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-service/preview/2021-02-04/client"
"github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-service/preview/2021-02-04/client/consul_service"

cloud_vault "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-service/preview/2020-11-25/client"
"github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-service/preview/2020-11-25/client/vault_service"
cloud_vault "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-service/stable/2020-11-25/client"
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for taking care of this!

nywilken
nywilken approved these changes Jan 24, 2022
View reviewed changes
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.

This looks good to me. Nicely done; especially with the added tests.

@sylviamoss sylviamoss force-pushed the hcp_packer_check_revoked_iteration branch from 37c6965 to 019d747 Compare January 25, 2022 14:05
@sylviamoss sylviamoss force-pushed the hcp_packer_check_revoked_iteration branch from 019d747 to 2e11ddd Compare January 25, 2022 14:16
bcmdarroch
bcmdarroch approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@bcmdarroch bcmdarroch left a comment

Choose a reason for hiding this comment

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

Once you get the Go SDK v0.16.0 in, this is good to go!

@sylviamoss sylviamoss changed the title A check for revoked iterations to HCP Packer datasources Add check for revoked iterations to HCP Packer datasources Jan 26, 2022
@sylviamoss sylviamoss merged commit ae9fddc into main Jan 26, 2022
@sylviamoss sylviamoss deleted the hcp_packer_check_revoked_iteration branch January 26, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcmdarroch bcmdarroch bcmdarroch approved these changes

@nywilken nywilken nywilken 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.
4 participants
@sylviamoss @kkredit @nywilken @bcmdarroch