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

VAULT-29412: Preview of new Vault Radar resource. #1092

Merged
merged 12 commits into from
Sep 24, 2024
Merged

Conversation

trentdibacco
Copy link
Contributor

@trentdibacco trentdibacco commented Sep 16, 2024

Preview of Manage a Vault Radar source of type GitHub Enterprise via the HCP Terraform Provider.

Important

This is a preview, and my intent is for this resource not to show up in the in the online documentations.
Therefore, I did not set the subcategory in the markup.

New Resource Checklist

  • Plugin Framework
  • Minimal LOC: 1 resource at a time.
  • Acceptance Tests:
  • Documentation:
  • Well-formed Code: go fmt.

Schema

  • Uses Globally Unique ID: Uses GUID returned by server.
  • Validates Fields Where Possible: Minimal
  • Supports Optional Project ID Input:

CRUD Operations

  • Uses Context-Aware CRUD Functions:
  • Uses Context For API Calls:
  • Uses DRY API Calls: added vault_radar under client dir for reusable client ops.
  • Handles Existing Resource Prior To Create: ? Server side will return an error during creation, no public api yet for checking prior to creating.
  • Implements Immediate Resource ID Set During Create: ? before other API operations or returning? Its using the GUID returned by the server side.
  • Refreshes Attributes During Read: most of these attributes are immutable.
  • Handles Removed Resource During Read:
  • Handles Failed State During Read:
  • Handles Nonexistent Resource Prior To Delete:

Documentation

  • Includes Descriptions For Resource And Fields:
  • Includes Example:
  • Includes Generated Docs:

🛠️ Description

Manage a Vault Radar source of type GitHub Enterprise via the HCP Terraform Provider.

🏗️ Acceptance tests

Warning

This AC test is being skipped because of pre-existing conditions that would be require for the test to succeed.

  • Requires Project already setup with Radar.
  • Requires a Service Account with an Admin role on the Project.
  • Requires access to a GitHub Enterprise Server self-managed instance.
  • Requires the following environment variables to be set:
    • HCP_PROJECT_ID
    • RADAR_GITHUB_ENTERPRISE_ORGANIZATION
    • RADAR_GITHUB_ENTERPRISE_DOMAIN
    • RADAR_GITHUB_ENTERPRISE_TOKEN

Note

I've created a ticket to note how we can address this sometime in the future.
https://hashicorp.atlassian.net/browse/VAULT-31051

  • 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=TestRadarSourceGitHubEnterprise'
==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml 
TF_ACC=1 go test ./internal/... -v -run=TestRadarSourceGitHubEnterprise -timeout 360m -parallel=10
?   	github.com/hashicorp/terraform-provider-hcp/internal/clients/iampolicy	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/clients/packerv2	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/customdiags	[no test files]
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/hcpvalidator	(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]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/acctest	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/customtypes	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/iam/helper	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/modifiers	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/iam	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/logstreaming	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/datasources/artifact	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/datasources/version	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/configbuilder	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/configbuilder/packerconfig	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/testcheck	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/testclient	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils/base	[no test files]
?   	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils/location	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/resources/bucket	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/resourcemanager	(cached) [no tests to run]
=== RUN   TestRadarSourceGitHubEnterprise
--- PASS: TestRadarSourceGitHubEnterprise (17.23s)
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/vaultradar	17.868s
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/vaultsecrets	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/waypoint	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/webhook	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider/webhook/validator	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/providersdkv2	(cached) [no tests to run]

...

Manage a Vault Radar source of type GitHub Enterprise via the HCP Terraform Provider.
Copy link

hashicorp-cla-app bot commented Sep 16, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

token := os.Getenv("RADAR_GITHUB_ENTERPRISE_TOKEN")

if projectID == "" || githubOrganization == "" || domainName == "" || token == "" {
t.Skip("HCP_PROJECT_ID, RADAR_GITHUB_ENTERPRISE_ORGANIZATION, RADAR_GITHUB_ENTERPRISE_DOMAIN, and RADAR_GITHUB_ENTERPRISE_TOKEN must be set for acceptance tests")
Copy link
Contributor Author

@trentdibacco trentdibacco Sep 16, 2024

Choose a reason for hiding this comment

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

@jasonpilz Seems the automated testing is on, and this will fail for CI.

  • Requires Project already setup with Radar.
  • Requires a Service Account with an Admin role on the Project.
  • Requires access to a GitHub Enterprise self-managed instance.

This is a best effort test, and works on my local setup with the proper env setup.

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 created the following ticket to try an address this in the future.
https://hashicorp.atlassian.net/browse/VAULT-31051

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, thanks 👍🏻

Copy link

@kathytong-hashicorp kathytong-hashicorp left a comment

Choose a reason for hiding this comment

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

Can't say I understand much of the TF stuff but the Radar stuff looks good to me!

Copy link

@kartheek-hc kartheek-hc left a comment

Choose a reason for hiding this comment

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

Looks good, have few some questions and suggestions

Update resource desc, fix context, better regex for validating domain name.
When the resource has been marked, but not fully deleted we need to handle
this case differently for Get and Delete.
@trentdibacco trentdibacco marked this pull request as ready for review September 17, 2024 21:15
@trentdibacco trentdibacco requested review from a team as code owners September 17, 2024 21:15
Copy link
Member

@jasonpilz jasonpilz left a comment

Choose a reason for hiding this comment

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

LGTM!

@trentdibacco trentdibacco requested a review from a team as a code owner September 24, 2024 18:15
@trentdibacco trentdibacco merged commit ef18399 into main Sep 24, 2024
6 checks passed
@trentdibacco trentdibacco deleted the radar-VAULT-29412 branch September 24, 2024 18:54
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.

4 participants