Add support for policy tool version in terraform providers #1202

mrinalirao · 2024-01-04T00:15:47Z

Description

Add support in the tfe provider for sentinel and OPA versions

Remember to:

Update the Change Log
Update the Documentation

Output from acceptance tests

Sentinel tests:
Test Config:
ENABLE_TFE=1;TF_ACC=1;TFE_HOSTNAME=tfcdev-1dfef877.au.ngrok.io;TFE_TOKEN=REDACTED

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___3TestAccTFESentinelVersion_basic_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFESentinelVersion_basic\E$
=== RUN   TestAccTFESentinelVersion_basic
--- PASS: TestAccTFESentinelVersion_basic (9.52s)
PASS

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___1TestAccTFESentinelVersion_import_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFESentinelVersion_import\E$
=== RUN   TestAccTFESentinelVersion_import
--- PASS: TestAccTFESentinelVersion_import (10.79s)
PASS

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___TestAccTFESentinelVersion_full_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFESentinelVersion_full\E$
=== RUN   TestAccTFESentinelVersion_full
--- PASS: TestAccTFESentinelVersion_full (8.12s)
PASS

OPA Tests:

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___9TestAccTFEOPAVersion_basic_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFEOPAVersion_basic\E$
=== RUN   TestAccTFEOPAVersion_basic
--- PASS: TestAccTFEOPAVersion_basic (7.94s)
PASS

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___TestAccTFEOPAVersion_import_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFEOPAVersion_import\E$
=== RUN   TestAccTFEOPAVersion_import
--- PASS: TestAccTFEOPAVersion_import (10.63s)
PASS

/usr/local/go/bin/go tool test2json -t /private/var/folders/dw/g3nt1z3d5sl4ll_r8f27ytwc0000gq/T/GoLand/___TestAccTFEOPAVersion_full_in_github_com_hashicorp_terraform_provider_tfe_internal_provider.test -test.v -test.paniconexit0 -test.run ^\QTestAccTFEOPAVersion_full\E$
=== RUN   TestAccTFEOPAVersion_full
--- PASS: TestAccTFEOPAVersion_full (7.99s)
PASS

Process finished with the exit code 0

Website Preview:

CHANGELOG.md

internal/provider/provider.go

website/docs/r/opa_version.html.markdown

internal/provider/resource_tfe_opa_version.go

internal/provider/resource_tfe_sentinel_version.go

laurenolivia · 2024-01-08T22:27:10Z

internal/provider/resource_tfe_opa_version.go

+
+	d.SetId(v.ID)
+
+	return resourceTFEOPAVersionUpdate(d, meta)


By convention, our Create and Update methods generally return the Read method. Can you share more about why this method returns Update?

Well... 😅 I was trying to follow the same approach as tfe_terraform_version provider as the policy version providers should be similar. Underneath the hood, they use the same tool_version code in atlas.

I briefly looked into it, and I think tfe_terraform_version is in error here (probably just a typo) and all three resources should be returning a Read. Sorry for the misleading crib sheet!

laurenolivia · 2024-01-08T22:29:12Z

internal/provider/resource_tfe_sentinel_version.go

+
+	d.SetId(v.ID)
+
+	return resourceTFESentinelVersionUpdate(d, meta)


Same here. Can you share more about why this method returns Update instead of Read?

same as above ^ #1202 (comment)

internal/provider/resource_tfe_opa_version.go

laurenolivia

Great work! Can you provide instructions on how to smoke test this?

mrinalirao · 2024-01-09T03:45:25Z

Smoke test instructions:

Run go build -gcflags="all=-N -l" -o bin/terraform-provider-tfe_v99.99.99_x5
Use the following config files to run against your local TFC
.terraformrc

provider_installation {
  dev_overrides {
    "hashicorp/tfe" = "<PATH>/terraform-provider-tfe/bin"
  }

  direct {}
}

main.tf

provider "tfe" {
  hostname = "tfcdev-1dfef877.au.ngrok.io" //link to localhost
  token = "<REDACTED>"
}

resource "tfe_sentinel_version" "foo" {
  version = "0.24.0"
  url = "https://releases.hashicorp.com/sentinel/0.24.0/sentinel_0.24.0_darwin_amd64.zip"
  sha = "1b1038817bab0b7a1180e2e3e5bca85f1e1fc7c5f52698ca15df408b67ef22df"
}

resource "tfe_opa_version" "bar" {
  version = "0.60.0"
  url = "https://github.com/open-policy-agent/opa/releases/download/v0.60.0/opa_linux_amd64"
  sha = "71514c6c70e744713656a302131e3172988c4898b43cb503f273086d47ccc299"
}

terraform {
  required_providers {
    tfe = {
      version = "~> 99.99.99"
    }
  }
}

Do a terraform login tfcdev-1dfef877.au.ngrok.io
Run terraform apply and see the resources get created

mrinalirao · 2024-01-11T02:33:28Z

Results of smoke test:

Ensure your admin user has version_maintaince role to be able to successfully hit the API's:

2024-01-11T13:26:50.264+1100 [WARN]  Provider "registry.terraform.io/hashicorp/tfe" produced an invalid plan for tfe_sentinel_version.foo, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .deprecated: planned value cty.False for a non-computed attribute
      - .beta: planned value cty.False for a non-computed attribute
      - .enabled: planned value cty.True for a non-computed attribute
      - .official: planned value cty.False for a non-computed attribute
tfe_sentinel_version.foo: Creating...
2024-01-11T13:26:50.264+1100 [INFO]  Starting apply for tfe_sentinel_version.foo
2024-01-11T13:26:50.264+1100 [DEBUG] tfe_sentinel_version.foo: applying the planned Create change
2024-01-11T13:26:50.264+1100 [WARN]  Provider "registry.terraform.io/hashicorp/tfe" produced an invalid plan for tfe_opa_version.bar, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .official: planned value cty.False for a non-computed attribute
      - .deprecated: planned value cty.False for a non-computed attribute
      - .enabled: planned value cty.True for a non-computed attribute
      - .beta: planned value cty.False for a non-computed attribute
      - .deprecated_reason: planned value cty.StringVal("") for a non-computed attribute
tfe_opa_version.bar: Modifying... [id=tool-m5QKtU912cRcJV95]
2024-01-11T13:26:50.264+1100 [INFO]  Starting apply for tfe_opa_version.bar
2024-01-11T13:26:50.264+1100 [DEBUG] provider.terraform-provider-tfe_v99.99.99_x5: [DEBUG] Create new Sentinel version: 0.24.6
2024-01-11T13:26:50.264+1100 [DEBUG] tfe_opa_version.bar: applying the planned Update change
2024-01-11T13:26:50.265+1100 [DEBUG] provider.terraform-provider-tfe_v99.99.99_x5: [DEBUG] Update configuration of OPA version: tool-m5QKtU912cRcJV95
2024-01-11T13:26:51.180+1100 [DEBUG] provider.terraform-provider-tfe_v99.99.99_x5: [DEBUG] Read configuration of OPA version: tool-m5QKtU912cRcJV95
2024-01-11T13:26:52.205+1100 [DEBUG] provider.terraform-provider-tfe_v99.99.99_x5: [DEBUG] Update configuration of Sentinel version: tool-mXmpPKvKXtgCgATE
tfe_opa_version.bar: Modifications complete after 2s [id=tool-m5QKtU912cRcJV95]
2024-01-11T13:26:52.230+1100 [DEBUG] State storage *statemgr.Filesystem declined to persist a state snapshot
2024-01-11T13:26:52.736+1100 [DEBUG] provider.terraform-provider-tfe_v99.99.99_x5: [DEBUG] Read configuration of Sentinel version: tool-mXmpPKvKXtgCgATE
2024-01-11T13:26:53.346+1100 [WARN]  Provider "provider[\"registry.terraform.io/hashicorp/tfe\"]" produced an unexpected new value for tfe_sentinel_version.foo, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .deprecated_reason: was null, but now cty.StringVal("")
tfe_sentinel_version.foo: Creation complete after 3s [id=tool-mXmpPKvKXtgCgATE]


Apply complete! Resources: 1 added, 1 changed, 0 destroyed.
mrinalirao@mrinalirao-V27QW1R72C .testing %

mrinalirao · 2024-01-11T02:35:26Z

Can also see the versions created by the provider in the admin UI

nfagerlund

All right, this is in good shape!

I ran into one piece of comedy while smoke testing -- it requires deprecated_reason to be present when creating a resource, but you can leave it blank when updating an existing one. But I think that's probably an atlas bug, I can't see any issue in this code that would have caused it.

I have two change requests:

Please return Read from Create (sorry, the other resource just seems to be in error).
Please add a note to the docs, ~~just because the site admin stuff is such a hazard.~~

nfagerlund · 2024-01-12T20:40:52Z

internal/provider/resource_tfe_opa_version.go

+
+	d.SetId(v.ID)
+
+	return resourceTFEOPAVersionUpdate(d, meta)


I briefly looked into it, and I think tfe_terraform_version is in error here (probably just a typo) and all three resources should be returning a Read. Sorry for the misleading crib sheet!

website/docs/r/opa_version.html.markdown

nfagerlund

Noticed one more thing that wants adjustment before merge. Apologies again for not noticing that before!

internal/provider/resource_tfe_opa_version.go

internal/provider/resource_tfe_sentinel_version.go

I followed up with mrinali and it looks like we caught all that feedback.

mrinalirao force-pushed the mr/policy_version_changes branch 2 times, most recently from 204051c to 0cbaba5 Compare January 4, 2024 03:51

mrinalirao marked this pull request as ready for review January 4, 2024 04:01

mrinalirao requested a review from a team as a code owner January 4, 2024 04:01

mrinalirao requested a review from dylanegan January 4, 2024 04:18

dylanegan reviewed Jan 4, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

mrinalirao force-pushed the mr/policy_version_changes branch from 161c478 to 358346d Compare January 5, 2024 05:15