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

HCE-339 feat multi project #454

Merged
merged 19 commits into from
May 23, 2023
Merged

Conversation

JolisaBrownHashiCorp
Copy link
Contributor

@JolisaBrownHashiCorp JolisaBrownHashiCorp commented Feb 9, 2023

🛠️ Description

This PR includes changes to support adding project ID at the provider level. When no project ID is specified, the provider selects as default the ID of the first attached project.

Jira 🎟️ - Add project ID to provider config

🏗️ 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=TestAccHvnOnly'

==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml 
WARN [linters context] bodyclose is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649. 
WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649. 
TF_ACC=1 go test ./internal/... -v -run=TestAccHvnOnly -timeout 360m -parallel=10
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/clients    (cached) [no tests to run]
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]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider   (cached) [no tests to run]

bcmdarroch and others added 18 commits May 12, 2023 12:35
* add validator for resource level project id, hce-339

* optionall read hvn project id from resource or provider config

* adds project id validator unit tests

* revise tests

* WIP - investigate multiproject acceptance testing

* multiproject unit tests (GTG), and acceptance tests (WIP)

* create helper to retrieve project ID

* use project ID helper when creating hvn

* fix linting/incorporate comments

* implement multiproject planonly acceptance test coverage

* swap validator for isUUID

* update datasource to support optional project id

* reduce variables

* add provider method to create project

* update documentation

* fix project err to ensure it returns complete err

* add project validator to provider, update test error match

---------

Co-authored-by: Brenna Hewer-Darroch <21015366+bcmdarroch@users.noreply.github.com>
* add project_id to consul resources

* typo

* typo
* add project id to aws network peering resource

* add project ID field to datasource

* update docs
* add project_id

* typo

* missing resource
* add multi proj guide template

* add TF examples

* regen doc

* update warning

* clarify default project when no project set
* allow explicit project id on import

* add project id to other resources

* linting and test coverage

* more explicit comments
@hashicorp-cla
Copy link

hashicorp-cla commented May 12, 2023

CLA assistant check
All committers have signed the CLA.

@delores-hashicorp delores-hashicorp marked this pull request as ready for review May 15, 2023 18:21
@delores-hashicorp delores-hashicorp requested a review from a team as a code owner May 15, 2023 18:21
@delores-hashicorp delores-hashicorp requested a review from a team May 15, 2023 18:21
@delores-hashicorp delores-hashicorp requested review from a team as code owners May 15, 2023 18:21

### Override provider project with resource project

Projects may be set at both the resource and provider level. The resource-configured project is always preferred over the provider-configured project.
Copy link

Choose a reason for hiding this comment

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

I discussed with @glasner about setting the project_id at the resource v.s. the provider config level in the use case of using multiple projects for multiple resources.
The general guidance from Jordan is to:

  • Use Provider Alias (docs here) to create aliases of providers with individual configurations (can have one alias per project). It would remove the need to assign projects to each resource, especially when there are multiple resources referencing different products (like HCP Packer and HCP Vault Secrets) to be created.

  • If there is only one HCP product resource with a limited resource count, like a few data sources referencing different HCP Vault Secret applications in different HCP projects, setting the project at the resource level is optimal.

Let me know if I missed anything in our conversation, @glasner .

Choose a reason for hiding this comment

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

To clarify: this is not a blocker, but a suggested change, to follow the Provider’s config best practice.

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.

:shipit: 🎉 😅

@delores-hashicorp delores-hashicorp changed the title Hce 339 feat multi project HCE-339 feat multi project May 16, 2023
@@ -65,6 +66,14 @@ func resourceVaultCluster() *schema.Resource {
ValidateDiagFunc: validateSlugID,
},
// Optional fields
"project_id": {
Description: "The ID of the HCP project where the Vault cluster is located.",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it'd useful to say that if this parameter is not specified then the oldest project will be targeted

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your feedback, I've created a ticket to address it in a separate PR. Since it is a large change, I will discuss it with the team and we will implement it separately.

@dmalch-hashicorp dmalch-hashicorp merged commit e4edb8e into main May 23, 2023
6 checks passed
@dmalch-hashicorp dmalch-hashicorp deleted the hce-339-feat-multi-project branch May 23, 2023 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet