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 revoke_at to HCP Packer datasources output #330

Merged
merged 5 commits into from
Jun 22, 2022
Merged

Add revoke_at to HCP Packer datasources output #330

merged 5 commits into from
Jun 22, 2022

Conversation

sylviamoss
Copy link
Contributor

@sylviamoss sylviamoss commented Jun 17, 2022 •
edited
Loading

🛠️ Description

HCP Packer will no longer return error_revoked so we are adding the revoke_at timestamp to all of our datasources.

🚢 Release Note

* datasource/hcp_packer_image: Include `revoke_at` in the data source output. [GH-330]
* datasource/hcp_packer_iteration: Include `revoke_at` in the data source output. [GH-330]
* datasource/hcp_packer_image_iteration: Include `revoke_at` in the data source output. [GH-330]

🏗️ 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:

==> Checking that code complies with gofmt requirements...
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      (cached) [no tests to run]
=== RUN   TestAcc_dataSourcePacker
--- PASS: TestAcc_dataSourcePacker (24.79s)
=== RUN   TestAcc_dataSourcePacker_revokedIteration
--- PASS: TestAcc_dataSourcePacker_revokedIteration (29.26s)
=== RUN   TestAcc_dataSourcePackerImage
--- PASS: TestAcc_dataSourcePackerImage (24.83s)
=== RUN   TestAcc_dataSourcePackerImage_revokedIteration
--- PASS: TestAcc_dataSourcePackerImage_revokedIteration (23.77s)
=== RUN   TestAcc_dataSourcePackerIteration
--- PASS: TestAcc_dataSourcePackerIteration (22.89s)
=== RUN   TestAcc_dataSourcePackerIteration_revokedIteration
--- PASS: TestAcc_dataSourcePackerIteration_revokedIteration (28.78s)
PASS

@sylviamoss sylviamoss marked this pull request as ready for review June 17, 2022 12:22
@sylviamoss sylviamoss requested a review from a team as a code owner June 17, 2022 12:22
@sylviamoss sylviamoss requested a review from a team June 17, 2022 12:22
bcmdarroch
bcmdarroch approved these changes Jun 17, 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.

Code looks good. I'm just curious why the value needs to be set on the provider side and not the backend.

internal/provider/data_source_packer_image_iteration.go
@@ -175,6 +181,11 @@ func dataSourcePackerImageIterationRead(ctx context.Context, d *schema.ResourceD
if err := d.Set("created_at", iteration.CreatedAt.String()); err != nil {
return diag.FromErr(err)
}
if !time.Time(iteration.RevokeAt).IsZero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this value only tracked in the datasource? Generally I would expect it to read from the API response

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iteration.RevokeAt is a value returned in API response. In this PR I'm making it available in the data source output. I do the zero check here, before writing it in the output, because the default value for strfmt.DateTime{} is 0001-01-01T00:00:00.000Z, so to avoid writing the zero value in the output.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh I misread this haha :doh:

nywilken
nywilken approved these changes Jun 21, 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 changes look good to me. One suggestion but otherwise looks good to go.

@sylviamoss sylviamoss merged commit 7d663ff into main Jun 22, 2022
@sylviamoss sylviamoss deleted the packer_datasources_add_revoke_at_output branch June 22, 2022 07:34
aidan-mundy pushed a commit that referenced this pull request Sep 8, 2023
@sylviamoss @bcmdarroch
Add revoke_at to HCP Packer datasources output (#330)
f060270
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.
3 participants
@sylviamoss @nywilken @bcmdarroch