From 4a40f9d910d3d20170c153f3ee167c1d187bf29a Mon Sep 17 00:00:00 2001 From: Salim Afiune Maya Date: Mon, 28 Oct 2024 05:46:19 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix=20inconsistent=20results=20a?= =?UTF-8?q?fter=20apply=20(#146)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the change where we allow customers to configure the `space` at the provider level, I found two issues when running Terraform apply: Error 1: Error: Provider returned invalid result object after apply After the apply operation, the provider still indicated an unknown value for mondoo_integration_slack.space.space_id. All values must be known after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known object values in the state. Error 2: Error: Provider produced inconsistent result after apply When applying changes to mondoo_integration_slack.space, provider "provider[\"registry.terraform.io/hashicorp/mondoo\"]" produced an unexpected new value: .space_id: was null, but now cty.StringVal("silly-hamilton-515695"). This is a bug in the provider, which should be reported in the provider's own issue tracker. This change fixes both of these issues. Note that we didn't catch this issue for the lack of acceptance tests on all integration resources, I added tests for two integrations, Slack and Shodan, but we probably need to do the rest of them soon. Signed-off-by: Salim Afiune Maya --- .../provider/custom_framework_resource.go | 4 + .../provider/framework_assignment_resource.go | 6 ++ internal/provider/gql.go | 2 + internal/provider/integration_aws_resource.go | 4 + .../integration_aws_serverless_resource.go | 4 + .../provider/integration_azure_resource.go | 4 + .../provider/integration_domain_resource.go | 4 + internal/provider/integration_gcp_resource.go | 4 + .../provider/integration_github_resource.go | 4 + .../provider/integration_ms365_resource.go | 4 + internal/provider/integration_oci_tenant.go | 4 + .../provider/integration_shodan_resource.go | 3 + .../integration_shodan_resource_test.go | 87 +++++++++++++++++++ .../provider/integration_slack_resource.go | 4 + .../integration_slack_resource_test.go | 81 +++++++++++++++++ internal/provider/provider_test.go | 75 +++++++++++++++- .../provider/querypack_assignment_resource.go | 5 ++ internal/provider/registration_token.go | 4 + internal/provider/service_account_resource.go | 3 + 19 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 internal/provider/integration_shodan_resource_test.go create mode 100644 internal/provider/integration_slack_resource_test.go diff --git a/internal/provider/custom_framework_resource.go b/internal/provider/custom_framework_resource.go index 4d646b5..1b19bab 100644 --- a/internal/provider/custom_framework_resource.go +++ b/internal/provider/custom_framework_resource.go @@ -78,6 +78,10 @@ func (r *customFrameworkResource) Schema(_ context.Context, _ resource.SchemaReq "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ MarkdownDescription: "Mondoo Resource Name.", diff --git a/internal/provider/framework_assignment_resource.go b/internal/provider/framework_assignment_resource.go index 4832f9c..6a1f6f6 100644 --- a/internal/provider/framework_assignment_resource.go +++ b/internal/provider/framework_assignment_resource.go @@ -7,6 +7,8 @@ import ( "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-log/tflog" ) @@ -41,6 +43,10 @@ func (r *frameworkAssignmentResource) Schema(_ context.Context, _ resource.Schem "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "framework_mrn": schema.ListAttribute{ MarkdownDescription: "Compliance Framework MRN.", diff --git a/internal/provider/gql.go b/internal/provider/gql.go index 48cff17..5915348 100644 --- a/internal/provider/gql.go +++ b/internal/provider/gql.go @@ -908,6 +908,8 @@ func (c *ExtendedGqlClient) DeleteFramework(ctx context.Context, mrn string) err // the provided MRN and if it exists, it compares the space configured at the provider level (if any). func (c *ExtendedGqlClient) ImportIntegration(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) (*Integration, bool) { mrn := req.ID + ctx = tflog.SetField(ctx, "mrn", mrn) + tflog.Debug(ctx, "importing integration") integration, err := c.GetClientIntegration(ctx, mrn) if err != nil { resp.Diagnostics. diff --git a/internal/provider/integration_aws_resource.go b/internal/provider/integration_aws_resource.go index 75861e6..59ade84 100644 --- a/internal/provider/integration_aws_resource.go +++ b/internal/provider/integration_aws_resource.go @@ -95,6 +95,10 @@ func (r *integrationAwsResource) Schema(ctx context.Context, req resource.Schema "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_aws_serverless_resource.go b/internal/provider/integration_aws_serverless_resource.go index 2498c35..5e8b1fa 100644 --- a/internal/provider/integration_aws_serverless_resource.go +++ b/internal/provider/integration_aws_serverless_resource.go @@ -211,6 +211,10 @@ func (r *integrationAwsServerlessResource) Schema(ctx context.Context, req resou "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_azure_resource.go b/internal/provider/integration_azure_resource.go index eb4fdf3..ba7e1e0 100644 --- a/internal/provider/integration_azure_resource.go +++ b/internal/provider/integration_azure_resource.go @@ -59,6 +59,10 @@ func (r *integrationAzureResource) Schema(ctx context.Context, req resource.Sche "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_domain_resource.go b/internal/provider/integration_domain_resource.go index 6ef53a3..d1af3ca 100644 --- a/internal/provider/integration_domain_resource.go +++ b/internal/provider/integration_domain_resource.go @@ -48,6 +48,10 @@ func (r *integrationDomainResource) Schema(ctx context.Context, req resource.Sch "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_gcp_resource.go b/internal/provider/integration_gcp_resource.go index 10bae07..9a8b31e 100644 --- a/internal/provider/integration_gcp_resource.go +++ b/internal/provider/integration_gcp_resource.go @@ -56,6 +56,10 @@ func (r *integrationGcpResource) Schema(ctx context.Context, req resource.Schema "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_github_resource.go b/internal/provider/integration_github_resource.go index f7d0c35..c37c7e8 100644 --- a/internal/provider/integration_github_resource.go +++ b/internal/provider/integration_github_resource.go @@ -95,6 +95,10 @@ func (r *integrationGithubResource) Schema(ctx context.Context, req resource.Sch "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_ms365_resource.go b/internal/provider/integration_ms365_resource.go index bc6146f..6b98012 100644 --- a/internal/provider/integration_ms365_resource.go +++ b/internal/provider/integration_ms365_resource.go @@ -55,6 +55,10 @@ func (r *integrationMs365Resource) Schema(ctx context.Context, req resource.Sche "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_oci_tenant.go b/internal/provider/integration_oci_tenant.go index 8f64758..776d79c 100644 --- a/internal/provider/integration_oci_tenant.go +++ b/internal/provider/integration_oci_tenant.go @@ -66,6 +66,10 @@ func (r *integrationOciTenantResource) Schema(ctx context.Context, req resource. "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_shodan_resource.go b/internal/provider/integration_shodan_resource.go index 0511b37..397a6e2 100644 --- a/internal/provider/integration_shodan_resource.go +++ b/internal/provider/integration_shodan_resource.go @@ -60,6 +60,9 @@ func (r *integrationShodanResource) Schema(_ context.Context, _ resource.SchemaR MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_shodan_resource_test.go b/internal/provider/integration_shodan_resource_test.go new file mode 100644 index 0000000..57d62bf --- /dev/null +++ b/internal/provider/integration_shodan_resource_test.go @@ -0,0 +1,87 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package provider + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func TestAccShodanResource(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + // Create and Read testing + { + Config: testAccShodanResourceConfig(accSpace.ID(), "one", []string{"mondoo.com"}), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "name", "one"), + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "space_id", accSpace.ID()), + ), + }, + { + Config: testAccShodanResourceWithSpaceInProviderConfig(accSpace.ID(), "two", "abctoken12345"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "name", "two"), + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "space_id", accSpace.ID()), + ), + }, + // ImportState testing + // @afiune this doesn't work since most of our resources doesn't have the `id` attribute + // if we add it, instead of the `mrn` or as a copy, this import test will work + // { + // ResourceName: "mondoo_integration_shodan.test", + // ImportState: true, + // ImportStateVerify: true, + // }, + // Update and Read testing + { + Config: testAccShodanResourceConfig(accSpace.ID(), "three", []string{"mondoo.com"}), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "name", "three"), + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "space_id", accSpace.ID()), + ), + }, + { + Config: testAccShodanResourceWithSpaceInProviderConfig(accSpace.ID(), "four", "0987xyzabc7654"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "name", "four"), + resource.TestCheckResourceAttr("mondoo_integration_shodan.test", "space_id", accSpace.ID()), + ), + }, + // Delete testing automatically occurs in TestCase + }, + }) +} + +func testAccShodanResourceConfig(spaceID, intName string, targets []string) string { + return fmt.Sprintf(` +resource "mondoo_integration_shodan" "test" { + space_id = %[1]q + name = %[2]q + targets = %[3]q + credentials = { + token = "abcd1234567890" + } +} +`, spaceID, intName, targets) +} + +func testAccShodanResourceWithSpaceInProviderConfig(spaceID, intName, token string) string { + return fmt.Sprintf(` +provider "mondoo" { + space = %[1]q +} +resource "mondoo_integration_shodan" "test" { + name = %[2]q + targets = ["8.8.8.8"] + credentials = { + token = %[3]q + } +} +`, spaceID, intName, token) +} diff --git a/internal/provider/integration_slack_resource.go b/internal/provider/integration_slack_resource.go index f1561e5..e4f22a9 100644 --- a/internal/provider/integration_slack_resource.go +++ b/internal/provider/integration_slack_resource.go @@ -49,6 +49,10 @@ func (r *integrationSlackResource) Schema(ctx context.Context, req resource.Sche "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/integration_slack_resource_test.go b/internal/provider/integration_slack_resource_test.go new file mode 100644 index 0000000..0f3c2a3 --- /dev/null +++ b/internal/provider/integration_slack_resource_test.go @@ -0,0 +1,81 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package provider + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func TestAccSlackResource(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + // Create and Read testing + { + Config: testAccSlackResourceConfig(accSpace.ID(), "one"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "name", "one"), + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "space_id", accSpace.ID()), + ), + }, + { + Config: testAccSlackResourceWithSpaceInProviderConfig(accSpace.ID(), "two"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "name", "two"), + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "space_id", accSpace.ID()), + ), + }, + // ImportState testing + // @afiune this doesn't work since most of our resources doesn't have the `id` attribute + // if we add it, instead of the `mrn` or as a copy, this import test will work + // { + // ResourceName: "mondoo_integration_slack.test", + // ImportState: true, + // ImportStateVerify: true, + // }, + // Update and Read testing + { + Config: testAccSlackResourceConfig(accSpace.ID(), "three"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "name", "three"), + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "space_id", accSpace.ID()), + ), + }, + { + Config: testAccSlackResourceWithSpaceInProviderConfig(accSpace.ID(), "four"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "name", "four"), + resource.TestCheckResourceAttr("mondoo_integration_slack.test", "space_id", accSpace.ID()), + ), + }, + // Delete testing automatically occurs in TestCase + }, + }) +} + +func testAccSlackResourceConfig(spaceID, intName string) string { + return fmt.Sprintf(` +resource "mondoo_integration_slack" "test" { + space_id = %[1]q + name = %[2]q + slack_token = "xoxa-1234567890abc" +} +`, spaceID, intName) +} + +func testAccSlackResourceWithSpaceInProviderConfig(spaceID, intName string) string { + return fmt.Sprintf(` +provider "mondoo" { + space = %[1]q +} +resource "mondoo_integration_slack" "test" { + name = %[2]q + slack_token = "xoxa-1234567890abc" +} +`, spaceID, intName) +} diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 0686680..6845f75 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -4,6 +4,7 @@ package provider import ( + "context" "encoding/base64" "encoding/json" "errors" @@ -13,8 +14,26 @@ import ( "github.com/hashicorp/terraform-plugin-framework/providerserver" "github.com/hashicorp/terraform-plugin-go/tfprotov6" + mondoov1 "go.mondoo.com/mondoo-go" + "go.mondoo.com/mondoo-go/option" ) +// Global space for those resources that need an existing space. +var accSpace Space + +func TestMain(m *testing.M) { + if err := createSpace(); err != nil { + panic(err) + } + + code := m.Run() + + if err := deleteSpace(); err != nil { + panic(err) + } + os.Exit(code) +} + type serviceAccountCredentials struct { Mrn string `json:"mrn,omitempty"` ParentMrn string `json:"parent_mrn,omitempty"` @@ -35,6 +54,57 @@ func testAccPreCheck(t *testing.T) { // nothing to do here for now } +func createSpace() error { + orgID, err := getOrgId() + if err != nil { + return err + } + + client, err := mondooClient() + if err != nil { + return err + } + extendedC := ExtendedGqlClient{client, ""} + + payload, err := extendedC.CreateSpace(context.Background(), orgID, "", "acceptance-test") + if err != nil { + return err + } + + accSpace = SpaceFrom(string(payload.Mrn)) + return nil +} + +func deleteSpace() error { + client, err := mondooClient() + if err != nil { + return err + } + extendedC := ExtendedGqlClient{client, ""} + + return extendedC.DeleteSpace(context.Background(), accSpace.ID()) +} + +func mondooClient() (*mondoov1.Client, error) { + if configBase64 := os.Getenv("MONDOO_CONFIG_BASE64"); configBase64 != "" { + // extract base 64 encoded string + data, err := base64.StdEncoding.DecodeString(configBase64) + if err != nil { + return nil, errors.New("MONDOO_CONFIG_BASE64 must be a valid service account") + } + + return mondoov1.NewClient(option.WithServiceAccount(data)) + } + + if configPath := os.Getenv("MONDOO_CONFIG_PATH"); configPath != "" { + + return mondoov1.NewClient(option.WithServiceAccountFile(configPath)) + } + return nil, errors.New( + "MONDOO_CONFIG_PATH or MONDOO_CONFIG_BASE64 must be a valid organization service account", + ) +} + func getOrgId() (string, error) { // You can add code here to run prior to any test case execution, for example assertions // about the appropriate environment variables being set are common to see in a pre-check @@ -72,6 +142,9 @@ func getOrgId() (string, error) { if len(m) == 2 { return m[1], nil } else { - return "", errors.New("MONDOO_CONFIG_PATH or MONDOO_CONFIG_BASE64 must be a valid organization service account") + return "", errors.New( + "MONDOO_CONFIG_PATH or MONDOO_CONFIG_BASE64 must be a valid organization service account", + ) + } } diff --git a/internal/provider/querypack_assignment_resource.go b/internal/provider/querypack_assignment_resource.go index 6ce3772..1b3993d 100644 --- a/internal/provider/querypack_assignment_resource.go +++ b/internal/provider/querypack_assignment_resource.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-log/tflog" mondoov1 "go.mondoo.com/mondoo-go" @@ -52,6 +53,10 @@ func (r *queryPackAssignmentResource) Schema(_ context.Context, req resource.Sch "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "querypacks": schema.ListAttribute{ MarkdownDescription: "QueryPacks to assign to the space.", diff --git a/internal/provider/registration_token.go b/internal/provider/registration_token.go index d863791..a976fe8 100644 --- a/internal/provider/registration_token.go +++ b/internal/provider/registration_token.go @@ -59,6 +59,10 @@ func (r *RegistrationTokenResource) Schema(ctx context.Context, req resource.Sch "space_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Space Identifier to create the token in. If it is not provided, the provider space is used.", Optional: true, + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "mrn": schema.StringAttribute{ Computed: true, diff --git a/internal/provider/service_account_resource.go b/internal/provider/service_account_resource.go index 82087ee..a7cfff1 100644 --- a/internal/provider/service_account_resource.go +++ b/internal/provider/service_account_resource.go @@ -102,6 +102,9 @@ func (r *ServiceAccountResource) Schema(ctx context.Context, req resource.Schema "space_id": schema.StringAttribute{ // TODO: add check that either space or org needs to be set MarkdownDescription: "Mondoo Space Identifier to create the service account in.", Optional: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "org_id": schema.StringAttribute{ MarkdownDescription: "Mondoo Organization Identifier to create the service account in.",