🐛 fix inconsistent results after apply (#146)

afiune · web-flow · commit 4a40f9d910d3 · 2024-10-28T05:46:19.000-07:00
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 &lt;afiune@mondoo.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/internal/provider/querypack_assignment_resource.go b/internal/provider/querypack_assignment_resource.go
diff --git a/internal/provider/registration_token.go b/internal/provider/registration_token.go
diff --git a/internal/provider/service_account_resource.go b/internal/provider/service_account_resource.go
