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

Add support for http observability on vault_cluster resource #660

Merged
merged 24 commits into from
Nov 7, 2023

Conversation

jaireddjawed
Copy link
Contributor

@jaireddjawed jaireddjawed commented Oct 30, 2023

🛠️ Description

This PR adds custom HTTP endpoints as an option for observability providers on HCP Vault clusters. I tested this change by adding a unit test that verifies that all required params that are a part of this provider are defined (located in internal/provider/resource_vault_cluster_config_test.go and performed a manual test of these changes.

Manual Test

Requirements

  • Terraform v1.6.2

Steps

  • Ran make dev in the main directory of this repository
  • Created the following terraform script in another folder
terraform {
  required_providers {
    hcp = {
      source  = "localhost/providers/hcp"
      version = "0.0.1"
    }
  }
}

provider "hcp" {
  client_id = "CLIENT_ID"
  client_secret = "CLIENT_SECRET"
  project_id = "PROJECT_ID"
}

provider "aws" {
  region="us-west-2"
}

// Create a HashiCorp Virtual Network (HVN):
resource "hcp_hvn" "terraform-hvn" {
  hvn_id         = "test-hvn"
  cloud_provider = "aws"
  region         = "us-west-2"
  cidr_block     = "172.25.16.0/20"
}

// Create HCPV cluster on AWS:
resource "hcp_vault_cluster" "tf-vault-cluster" {
  depends_on      = [hcp_hvn.terraform-hvn]
  hvn_id          = hcp_hvn.terraform-hvn.hvn_id
  cluster_id      = "test-vault-cluster"
  tier            = "standard_small"
  public_endpoint = true
  proxy_endpoint = "ENABLED"
  lifecycle {
    prevent_destroy = true
  }
  audit_log_config {
    http_uri="https://webhook.site/3eceb6c7-1007-4ce8-900a-fc41d1a400c9"
    http_codec="JSON"
    http_method="POST"
    http_headers = {
      "key1": "value1",
      "key2": "value2",
      "key3": "value3"
    }
    http_bearer_token = "111234567"
  }
}
  • Ran the script above by running terraform init and terraform apply in the directory of the terraform script
  • Verified that the script worked by looking at the created vault cluster within the portal and viewing that its audit logs provider is the http endpoint specified in the terraform script.
Screenshot 2023-11-01 at 11 35 08 AM
  • Checked to see if audit log http requests are the type of request specified in the terraform script (POST), that the headers and bearer token are seen in the http request
Screenshot 2023-11-01 at 11 38 45 AM

Verified that http observability information appears in /etc/vector.d/vector.json

Screenshot 2023-11-01 at 1 33 03 PM

Check to see if errors are valid

  • Verified that an error occurs when http_basic_user is given, but http_basic_password is not specified
audit_log_config {
    http_uri="https://webhook.site/3eceb6c7-1007-4ce8-900a-fc41d1a400c9"
    http_codec="JSON"
    http_method="POST"
    http_headers = {
      "key1": "value1",
      "key2": "value2",
      "key3": "value3"
    }
    http_basic_user = "user"
}
Screenshot 2023-11-01 at 11 36 46 AM
  • Verified that an error occurs when http_bearer_token and http_basic_user are specified
audit_log_config {
    http_uri="https://webhook.site/3eceb6c7-1007-4ce8-900a-fc41d1a400c9"
    http_codec="JSON"
    http_method="POST"
    http_headers = {
      "key1": "value1",
      "key2": "value2",
      "key3": "value3"
    }
    http_bearer_token = "1231223"
    http_basic_user = "user"
  }
Screenshot 2023-11-01 at 11 42 13 AM

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

...

@jaireddjawed jaireddjawed requested a review from a team as a code owner October 30, 2023 21:25
@jaireddjawed jaireddjawed marked this pull request as draft October 30, 2023 21:25
@jaireddjawed jaireddjawed self-assigned this Oct 30, 2023
@jaireddjawed jaireddjawed marked this pull request as ready for review October 30, 2023 21:37
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
docs/resources/vault_cluster.md Outdated Show resolved Hide resolved
docs/resources/vault_cluster.md Outdated Show resolved Hide resolved
to invalidProviderConfigError
…o specifiy that only basic or bearer authentication

can be provided at any given time
should only be "JSON" or "NDJSON" values
that its value is either POST, PUT, or PATCH
…rm-provider-hcp into jaireddjawed-hcpv-http

Merge changes from main
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
internal/providersdkv2/resource_vault_cluster.go Outdated Show resolved Hide resolved
Copy link
Contributor

@bosouza bosouza left a comment

Choose a reason for hiding this comment

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

lgtm

@jaireddjawed jaireddjawed merged commit 713bf23 into main Nov 7, 2023
6 checks passed
@jaireddjawed jaireddjawed deleted the jaireddjawed-hcpv-http branch November 7, 2023 19:50
@jaireddjawed jaireddjawed restored the jaireddjawed-hcpv-http branch November 7, 2023 19:51
jaireddjawed added a commit that referenced this pull request Nov 7, 2023
aidan-mundy pushed a commit that referenced this pull request Nov 9, 2023
aidan-mundy pushed a commit that referenced this pull request Nov 9, 2023
…#669)

* Revert "Add support for http observability on vault_cluster resource (#660)"

This reverts commit 713bf23.

* Create 669.txt

* Deleted unecessary changelog
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.

3 participants