[HPR-859] Add HCP Packer bucket channel resource #435

nywilken · 2022-12-19T21:58:00Z

🛠️ Description

This change adds the hcp_packer_bucket_channel resource to manage channels within an active HCP Packer registry image bucket.

🏗️ 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=TestAccPackerBucketChannel'
==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml
TF_ACC=1 go test ./internal/... -v -run=TestAccPackerBucketChannel -timeout 210m
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/clients    0.169s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/consul     0.126s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      0.088s [no tests to run]
=== RUN   TestAccPackerBucketChannel
--- PASS: TestAccPackerBucketChannel (9.75s)
=== RUN   TestAccPackerBucketChannel_AssignedIteration
--- PASS: TestAccPackerBucketChannel_AssignedIteration (11.78s)
=== RUN   TestAccPackerBucketChannel_UpdateAssignedIteration
--- PASS: TestAccPackerBucketChannel_UpdateAssignedIteration (14.74s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider   36.604s

...

Few Examples

# Create channel with no assigned iteration
resource "hcp_packer_channel" "staging" {
  name        = "staging"
  bucket_name = "alpine"
}

# Assign iteration to channel via the iteration ID. 
resource "hcp_packer_channel" "staging" {
  name        = "staging"
  bucket_name = "alpine"
  iteration_assignment {
    id = "iteration-id"
  }
}

# Update assigned iteration using an iteration fingerprint
resource "hcp_packer_channel" "staging" {
  name        = "staging"
  bucket_name = "alpine"
  iteration_assignment {
    fingerprint = "fingerprint-associated-to-iteration"
  }
}

# Update assigned iteration using an iteration incremental version
resource "hcp_packer_channel" "staging" {
  name        = "staging"
  bucket_name = "alpine"
  iteration_assignment {
    // incremental_version is the version number assigned to a completed iteration.
    incremental_version = 1
  }
}

internal/provider/resource_packer_channel.go

sylviamoss

I had a couple of questions and suggestions, but overall LGTM! 👍🏼

.changelog/435.txt

docs/resources/packer_channel.md

internal/provider/resource_packer_channel.go

internal/clients/packer.go

sylviamoss · 2023-01-25T11:12:55Z

docs/resources/packer_channel.md

+
+Read-Only:
+
+- `fingerprint` (String)


I wonder why these are not including the description you gave them 🤔

This seems to be a limitation of the tfplugindocs command when it comes to nested computed attributes.

@bcmdarroch Any thoughts on why the docs don't get generated for these nested attributes?

Yeah this is a known issue: hashicorp/terraform-plugin-docs#28. Unfortunately can't add further detail. :( The provider SDK doesn't have great support for nested objects in general.

Not sure if this would resolve this specific doc issue, but the new plugin framework has much better support for nested objects: https://developer.hashicorp.com/terraform/plugin/framework/migrating/benefits

internal/provider/resource_packer_channel.go

sylviamoss · 2023-01-25T11:18:52Z

internal/provider/resource_packer_channel_test.go

+				},
+				Config: testConfig(testAccPackerChannelAssignedLatestIteration(acctestAlpineBucket, acctestChannelName)),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet(resourceName, "iteration_assignment.0.id"),


What's this 0?

This is a block is a list type so to reference the attributes for the first element in the list, which is the only element, we have to index the results to get the values - https://developer.hashicorp.com/terraform/plugin/sdkv2/schemas/schema-types#typelist

lbajolet-hashicorp

Overall code looks good to me, I left a few comments, feel free to address them as you find relevant.

On another note, is there a reason why the terraform-provider-hcp_v0.51.0 binary is in the diff?

internal/provider/resource_packer_channel.go

lbajolet-hashicorp · 2023-01-25T17:05:34Z

internal/provider/resource_packer_channel.go

+	return nil
+}
+
+func resourcePackerChannelImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {


I'm not super familiar with the codebase here, but is there a reason why meta is an interface{} if we always expect it to be a *clients.Client?

Good question. The interface is to fulfill the function signature expected by the underlying terraform plugin SDK: in this case, the StateContextFunc. The meta argument provides access to the provider state, but generally resources only need the client out of it.

thanks @bcmdarroch I was struggling to find how to word this without going too deep. 🙇‍♂️

internal/provider/resource_packer_channel_test.go

lbajolet-hashicorp

Just left a final doc nit, but other than that, LGTM!

internal/clients/packer.go

nywilken · 2023-01-26T21:57:48Z

@bcmdarroch could I get your review on this, as well. I would like to know if there are any thoughts or suggestion on the TF schema.

bcmdarroch · 2023-01-27T20:30:38Z

internal/provider/resource_packer_channel_test.go

+}
+
+func TestAccPackerChannel_UpdateAssignedIterationWithFingerprint(t *testing.T) {
+	resourceName := "hcp_packer_channel.staging"


This test is failing for me:

terraform-provider-hcp (nywilken/add-packer-channel-resource) ✗ (~˘▾˘)~ ✧ make testacc TESTARGS='-run=PackerChannel' ==> Checking that code complies with gofmt requirements... golangci-lint run --config ./golangci-config.yml TF_ACC=1 go test ./internal/... -v -run=PackerChannel -timeout 210m -parallel=10 testing: warning: no tests to run === RUN TestAccPackerChannel --- PASS: TestAccPackerChannel (10.50s) === RUN TestAccPackerChannel_AssignedIteration --- PASS: TestAccPackerChannel_AssignedIteration (18.21s) === RUN TestAccPackerChannel_UpdateAssignedIteration --- PASS: TestAccPackerChannel_UpdateAssignedIteration (14.24s) === RUN TestAccPackerChannel_UpdateAssignedIterationWithFingerprint resource_packer_channel_test.go:181: Step 1/1 error: Check failed: Check 1/10 error: Not found: hcp_packer_channel.staging in [root] --- FAIL: TestAccPackerChannel_UpdateAssignedIterationWithFingerprint (4.18s) FAIL FAIL github.com/hashicorp/terraform-provider-hcp/internal/provider 47.876s FAIL

Thanks for catching this. It should be fixed now.

bcmdarroch · 2023-01-27T20:33:25Z

internal/provider/resource_packer_channel_test.go

+
+func TestAccPackerChannel_UpdateAssignedIterationWithFingerprint(t *testing.T) {
+	resourceName := "hcp_packer_channel.staging"
+	acctestChannelName := "fingerprint-test"


We've had some trouble with resource ID conflicts with our releaser acceptance test runs. Could you add a timestamp to this ID and the others? For example: https://github.com/hashicorp/terraform-provider-hcp/pull/440/files#diff-793cec3def4f921f6b01a086a16e6c0fb145f56b29613da7b36e9b10bcb4c922R17

Thanks for this info. I'll update.

bcmdarroch

Looking good! 🎉 Had a suggestion about timestamped IDs, and noticed that TestAccPackerChannel_UpdateAssignedIterationWithFingerprint failed when I ran the tests against integration.

nywilken · 2023-01-27T21:04:35Z

internal/provider/data_source_packer_image_iteration_test.go

@@ -22,7 +22,7 @@ import (
 var (
 	acctestAlpineBucket      = fmt.Sprintf("alpine-acc-%s", time.Now().Format("200601021504"))
 	acctestUbuntuBucket      = fmt.Sprintf("ubuntu-acc-%s", time.Now().Format("200601021504"))
-	acctestProductionChannel = "production"
+	acctestProductionChannel = fmt.Sprintf("packer-acc-channel-%s", time.Now().Format("200601021504"))


Validated all test pass as expected

~> make testacc TESTARGS='-run=Packer' ==> Checking that code complies with gofmt requirements... golangci-lint run --config ./golangci-config.yml TF_ACC=1 go test ./internal/... -v -run=Packer -timeout 210m -parallel=10 testing: warning: no tests to run PASS ok github.com/hashicorp/terraform-provider-hcp/internal/clients 0.216s [no tests to run] testing: warning: no tests to run PASS ok github.com/hashicorp/terraform-provider-hcp/internal/consul 0.302s [no tests to run] testing: warning: no tests to run PASS ok github.com/hashicorp/terraform-provider-hcp/internal/input 0.283s [no tests to run] === RUN TestAcc_dataSourcePacker --- PASS: TestAcc_dataSourcePacker (12.48s) === RUN TestAcc_dataSourcePacker_revokedIteration --- PASS: TestAcc_dataSourcePacker_revokedIteration (16.62s) === RUN TestAcc_dataSourcePackerImage --- PASS: TestAcc_dataSourcePackerImage (11.65s) === RUN TestAcc_dataSourcePackerImage_revokedIteration --- PASS: TestAcc_dataSourcePackerImage_revokedIteration (14.44s) === RUN TestAcc_dataSourcePackerImage_channelAndIterationIDReject --- PASS: TestAcc_dataSourcePackerImage_channelAndIterationIDReject (5.60s) === RUN TestAcc_dataSourcePackerImage_channelAccept --- PASS: TestAcc_dataSourcePackerImage_channelAccept (10.24s) === RUN TestAcc_dataSourcePackerIteration --- PASS: TestAcc_dataSourcePackerIteration (9.56s) === RUN TestAcc_dataSourcePackerIteration_revokedIteration --- PASS: TestAcc_dataSourcePackerIteration_revokedIteration (16.30s) === RUN TestAccPackerChannel --- PASS: TestAccPackerChannel (12.18s) === RUN TestAccPackerChannel_AssignedIteration --- PASS: TestAccPackerChannel_AssignedIteration (13.25s) === RUN TestAccPackerChannel_UpdateAssignedIteration --- PASS: TestAccPackerChannel_UpdateAssignedIteration (21.26s) === RUN TestAccPackerChannel_UpdateAssignedIterationWithFingerprint --- PASS: TestAccPackerChannel_UpdateAssignedIterationWithFingerprint (10.44s) PASS ok github.com/hashicorp/terraform-provider-hcp/internal/provider 154.408s

* Add ability to update channel assignment * Add validation for channel and bucket names

* Add author_id to output * Add updated_at computed property

The current implementation relies on the use of an extra iteration_assignment block to denote what iteration to assign to a channel. Instead of defining a separate block this change uses the single iteration block to denote the state of a channel. Modifying the iteration block will trigger the respective updates to the state of a channel. Removing the iteration block will remove the assigned iteration from the channel.

bcmdarroch

One more test run before final merge ✅

--- PASS: TestAccPackerChannel_UpdateAssignedIterationWithFingerprint (6.07s)
--- PASS: TestAccPackerChannel_UpdateAssignedIteration (13.34s)
--- PASS: TestAccPackerChannel_AssignedIteration (9.61s)
--- PASS: TestAccPackerChannel (7.26s)
--- PASS: TestAcc_dataSourcePackerIteration_revokedIteration (11.31s)
--- PASS: TestAcc_dataSourcePackerIteration (5.99s)
--- PASS: TestAcc_dataSourcePackerImage_channelAccept (6.21s)
--- PASS: TestAcc_dataSourcePackerImage_channelAndIterationIDReject (3.58s)
--- PASS: TestAcc_dataSourcePackerImage_revokedIteration (10.80s)
--- PASS: TestAcc_dataSourcePackerImage (7.79s)
--- PASS: TestAcc_dataSourcePacker_revokedIteration (11.53s)
--- PASS: TestAcc_dataSourcePacker (12.89s)

nywilken · 2023-02-09T21:22:44Z

One more test as well

~>  make testacc TESTARGS='-run=Packer'
==> Checking that code complies with gofmt requirements...
=== RUN   TestAcc_dataSourcePacker
--- PASS: TestAcc_dataSourcePacker (10.34s)
=== RUN   TestAcc_dataSourcePacker_revokedIteration
--- PASS: TestAcc_dataSourcePacker_revokedIteration (12.61s)
=== RUN   TestAcc_dataSourcePackerImage
--- PASS: TestAcc_dataSourcePackerImage (10.43s)
=== RUN   TestAcc_dataSourcePackerImage_revokedIteration
--- PASS: TestAcc_dataSourcePackerImage_revokedIteration (11.37s)
=== RUN   TestAcc_dataSourcePackerImage_channelAndIterationIDReject
--- PASS: TestAcc_dataSourcePackerImage_channelAndIterationIDReject (3.94s)
=== RUN   TestAcc_dataSourcePackerImage_channelAccept
--- PASS: TestAcc_dataSourcePackerImage_channelAccept (6.90s)
=== RUN   TestAcc_dataSourcePackerIteration
--- PASS: TestAcc_dataSourcePackerIteration (6.27s)
=== RUN   TestAcc_dataSourcePackerIteration_revokedIteration
--- PASS: TestAcc_dataSourcePackerIteration_revokedIteration (12.06s)
=== RUN   TestAccPackerChannel
--- PASS: TestAccPackerChannel (8.02s)
=== RUN   TestAccPackerChannel_AssignedIteration
--- PASS: TestAccPackerChannel_AssignedIteration (10.99s)
=== RUN   TestAccPackerChannel_UpdateAssignedIteration
--- PASS: TestAccPackerChannel_UpdateAssignedIteration (14.91s)
=== RUN   TestAccPackerChannel_UpdateAssignedIterationWithFingerprint
--- PASS: TestAccPackerChannel_UpdateAssignedIterationWithFingerprint (7.36s)
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider	115.589s

nywilken changed the title ~~Nywilken/add packer channel resource~~ [HPR-859] Add HCP Packer bucket channel resource Dec 19, 2022

nywilken force-pushed the nywilken/add-packer-channel-resource branch 6 times, most recently from 40f0f9e to 9f05312 Compare December 21, 2022 16:25

nywilken force-pushed the nywilken/add-packer-channel-resource branch from 9f05312 to 847e9ba Compare January 23, 2023 17:17

nywilken marked this pull request as ready for review January 24, 2023 23:13

nywilken requested a review from a team as a code owner January 24, 2023 23:13

nywilken requested a review from a team January 24, 2023 23:13

nywilken requested a review from a team as a code owner January 24, 2023 23:13

nywilken force-pushed the nywilken/add-packer-channel-resource branch from cfeb5c0 to 441134b Compare January 24, 2023 23:22

nywilken commented Jan 25, 2023

View reviewed changes

internal/provider/resource_packer_channel.go Show resolved Hide resolved

sylviamoss approved these changes Jan 25, 2023

View reviewed changes

lbajolet-hashicorp approved these changes Jan 25, 2023

View reviewed changes

nywilken force-pushed the nywilken/add-packer-channel-resource branch 3 times, most recently from 6e2120e to e31543e Compare January 26, 2023 19:45

lbajolet-hashicorp approved these changes Jan 26, 2023

View reviewed changes

internal/clients/packer.go Outdated Show resolved Hide resolved

nywilken requested a review from bcmdarroch January 26, 2023 21:57

nywilken force-pushed the nywilken/add-packer-channel-resource branch from 7bd7d47 to f606862 Compare January 26, 2023 22:24

bcmdarroch reviewed Jan 27, 2023

View reviewed changes

nywilken force-pushed the nywilken/add-packer-channel-resource branch from e337fff to f066cc1 Compare January 27, 2023 20:58

nywilken commented Jan 27, 2023

View reviewed changes

bcmdarroch approved these changes Jan 27, 2023

View reviewed changes

nywilken mentioned this pull request Jan 31, 2023

[HPR-859] Update resource schema to use a single iteration block #446

Merged

3 tasks

nywilken added 4 commits February 9, 2023 11:46

Add working create/delete resource

76bb4d0

* Add ability to update channel assignment * Add validation for channel and bucket names

Initial support for resource import

22508b3

Add other computed properties

34e2d38

* Add author_id to output * Add updated_at computed property

Update changelog

76b872b

nywilken force-pushed the nywilken/add-packer-channel-resource branch from f066cc1 to 76b872b Compare February 9, 2023 16:50

bcmdarroch approved these changes Feb 9, 2023

View reviewed changes

nywilken merged commit 99e5cb7 into main Feb 9, 2023

nywilken deleted the nywilken/add-packer-channel-resource branch February 9, 2023 21:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HPR-859] Add HCP Packer bucket channel resource #435

[HPR-859] Add HCP Packer bucket channel resource #435

nywilken commented Dec 19, 2022 •

edited

Loading

sylviamoss left a comment

sylviamoss Jan 25, 2023

nywilken Jan 26, 2023

nywilken Jan 26, 2023

bcmdarroch Jan 27, 2023

sylviamoss Jan 25, 2023

nywilken Jan 26, 2023

lbajolet-hashicorp left a comment

lbajolet-hashicorp Jan 25, 2023

bcmdarroch Jan 27, 2023

nywilken Jan 27, 2023

lbajolet-hashicorp left a comment

nywilken commented Jan 26, 2023

bcmdarroch Jan 27, 2023

nywilken Jan 27, 2023

bcmdarroch Jan 27, 2023

nywilken Jan 27, 2023

bcmdarroch left a comment

nywilken Jan 27, 2023

bcmdarroch Jan 27, 2023

bcmdarroch left a comment •

edited

Loading

nywilken commented Feb 9, 2023

[HPR-859] Add HCP Packer bucket channel resource #435

[HPR-859] Add HCP Packer bucket channel resource #435

Conversation

nywilken commented Dec 19, 2022 • edited Loading

🛠️ Description

🏗️ Acceptance tests

sylviamoss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

nywilken commented Jan 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcmdarroch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcmdarroch left a comment • edited Loading

Choose a reason for hiding this comment

nywilken commented Feb 9, 2023

nywilken commented Dec 19, 2022 •

edited

Loading

bcmdarroch left a comment •

edited

Loading