[terraform] Fix kubernetes_resources defaults

integrations/terraform/protoc-gen-terraform-teleport.yaml

@@ -445,6 +445,8 @@ plan_modifiers:
       - "github.com/hashicorp/terraform-plugin-framework/tfsdk.UseStateForUnknown()"
     RoleV6.Spec.Options:
       - "DefaultRoleOptions()"
+    RoleV6.Spec.Allow.KubernetesResources:
+      - "DefaultKubernetesResources()"
 
 validators:
   # Expires must be in the future

integrations/terraform/testlib/fixtures/role_upgrade_v6_with_kube_resources.tf

@@ -0,0 +1,23 @@
+resource "teleport_role" "upgrade" {
+  metadata = {
+    name = "upgrade"
+  }
+
+  spec = {
+    allow = {
+      logins = ["onev6"]
+      kubernetes_labels = {
+        env = ["dev", "prod"]
+      }
+      kubernetes_resources = [
+        {
+          kind      = "pod"
+          name      = "*"
+          namespace = "myns"
+        }
+      ]
+    }
+  }
+
+  version = "v6"
+}

integrations/terraform/testlib/fixtures/role_upgrade_v7_with_kube_resources.tf

@@ -0,0 +1,27 @@
+
+resource "teleport_role" "upgrade" {
+  metadata = {
+    name = "upgrade"
+  }
+
+  spec = {
+    allow = {
+      logins = ["onev7"]
+
+      kubernetes_labels = {
+        env = ["dev", "prod"]
+      }
+
+      kubernetes_resources = [
+        {
+          kind      = "deployment"
+          name      = "*"
+          namespace = "myns"
+          verbs     = ["get"]
+        }
+      ]
+    }
+  }
+
+  version = "v7"
+}

integrations/terraform/testlib/role_test.go

@@ -296,13 +296,6 @@ func (s *TerraformSuiteOSS) TestRoleLoginsSplitBrain() {
 }
 
 func (s *TerraformSuiteOSS) TestRoleVersionUpgrade() {
-	// TODO(hugoShaka) Re-enable this test when we fix the role defaults in v16
-	// We had a bug in v14 and below that caused the defaults to be badly computed.
-	// We tried to fix this bug in v15 but it was too aggressive (forcing replacement is too destructive).
-	// In v16 we'll push a new plan modifier to fix this issue, this might be a
-	// breaking change for users who relied on the bug.
-	s.T().Skip("Test temporarily disabled until v16")
-
 	ctx, cancel := context.WithCancel(context.Background())
 	s.T().Cleanup(cancel)
 
@@ -345,7 +338,7 @@ func (s *TerraformSuiteOSS) TestRoleVersionUpgrade() {
 		},
 	}
 
-	customWildcard := []types.KubernetesResource{
+	customV6KubeResources := []types.KubernetesResource{
 		{
 			Kind:      types.KindKubePod,
 			Namespace: "myns",
@@ -354,6 +347,15 @@ func (s *TerraformSuiteOSS) TestRoleVersionUpgrade() {
 		},
 	}
 
+	customV7KubeResources := []types.KubernetesResource{
+		{
+			Kind:      types.KindKubeDeployment,
+			Namespace: "myns",
+			Name:      types.Wildcard,
+			Verbs:     []string{types.KubeVerbGet},
+		},
+	}
+
 	checkRoleResource := func(version string, expected []types.KubernetesResource) resource.TestCheckFunc {
 		return func(state *terraform.State) error {
 			role, err := s.client.GetRole(ctx, "upgrade")
@@ -424,19 +426,19 @@ func (s *TerraformSuiteOSS) TestRoleVersionUpgrade() {
 				PlanOnly: true,
 			},
 			{
-				Config: s.getFixture("role_with_kube_resources.tf"),
+				Config: s.getFixture("role_upgrade_v6_with_kube_resources.tf"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(name, "kind", "role"),
 					resource.TestCheckResourceAttr(name, "version", "v6"),
 					resource.TestCheckResourceAttr(name, "spec.allow.logins.0", "onev6"),
 					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.kind", "pod"),
 					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.name", "*"),
 					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.namespace", "myns"),
-					checkRoleResource(types.V6, customWildcard),
+					checkRoleResource(types.V6, customV6KubeResources),
 				),
 			},
 			{
-				Config:   s.getFixture("role_with_kube_resources.tf"),
+				Config:   s.getFixture("role_upgrade_v6_with_kube_resources.tf"),
 				PlanOnly: true,
 			},
 			{
@@ -452,6 +454,23 @@ func (s *TerraformSuiteOSS) TestRoleVersionUpgrade() {
 				Config:   s.getFixture("role_upgrade_v7.tf"),
 				PlanOnly: true,
 			},
+			{
+				Config: s.getFixture("role_upgrade_v7_with_kube_resources.tf"),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(name, "kind", "role"),
+					resource.TestCheckResourceAttr(name, "version", "v7"),
+					resource.TestCheckResourceAttr(name, "spec.allow.logins.0", "onev7"),
+					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.kind", "deployment"),
+					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.name", "*"),
+					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.namespace", "myns"),
+					resource.TestCheckResourceAttr(name, "spec.allow.kubernetes_resources.0.verbs.0", "get"),
+					checkRoleResource(types.V7, customV7KubeResources),
+				),
+			},
+			{
+				Config:   s.getFixture("role_upgrade_v7_with_kube_resources.tf"),
+				PlanOnly: true,
+			},
 			{
 				Config: s.getFixture("role_upgrade_v8.tf"),
 				Check: resource.ComposeTestCheckFunc(

integrations/terraform/tfschema/role_plan_modifier.go

@@ -27,9 +27,10 @@ import (
 )
 
 const (
-	DefaultRoleOptionsModifierErrSummary  = "DefaultRoleOptions modifier failed"
-	DefaultRoleOptionsModifierDescription = `This modifier re-render the role.spec.options from the user provided config instead of using the state.
-The state contains server-generated defaults (in fact they are generated in the pre-apply plan).
+	DefaultRoleOptionsModifierErrSummary         = "DefaultRoleOptions modifier failed"
+	DefaultKubernetesResourcesModifierErrSummary = "DefaultKubernetesResources modifier failed"
+
+	DefaultModiferDescription = `The state contains server-generated defaults (in fact they are generated in the pre-apply plan).
 However, those defaults become outdated if the version or the default logic changes.
 One way to deal with version change is to force-recreate, but this is too destructive.
 The workaround we found was to use this plan modifier.`
@@ -47,12 +48,14 @@ type DefaultRoleOptionsModifier struct {
 
 // Description of the RoleOptions plan modifier
 func (d DefaultRoleOptionsModifier) Description(ctx context.Context) string {
-	return DefaultRoleOptionsModifierDescription
+	return "This modifier re-renders the role.spec.options from the user provided config instead of using the state. " +
+		DefaultModiferDescription
 }
 
 // MarkdownDescription of the RoleOptions plan modifier
 func (d DefaultRoleOptionsModifier) MarkdownDescription(ctx context.Context) string {
-	return DefaultRoleOptionsModifierDescription
+	return "This modifier re-renders the role.spec.options from the user provided config instead of using the state. " +
+		DefaultModiferDescription
 }
 
 // Modify the terraform plan to account for defaults applied to RoleOptions by CheckAndSetDefaults
@@ -82,7 +85,7 @@ func (d DefaultRoleOptionsModifier) Modify(ctx context.Context, req tfsdk.Modify
 	diags = CopyRoleV6ToTerraform(ctx, role, &config)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
-		resp.Diagnostics.AddError(DefaultRoleOptionsModifierErrSummary, "Failed to convert back the role into a TF Object.")
+		resp.Diagnostics.AddError(DefaultRoleOptionsModifierErrSummary, "Failed to convert back the role into a TF object.")
 		return
 	}
 
@@ -104,7 +107,98 @@ func (d DefaultRoleOptionsModifier) Modify(ctx context.Context, req tfsdk.Modify
 	options, ok := optionsRaw.(tftypes.Object)
 	if !ok {
 		resp.Diagnostics.AddError(DefaultRoleOptionsModifierErrSummary, "Failed to cast 'options' as a TF object.")
+		return
 	}
 	options.Null = false
 	resp.AttributePlan = options
 }
+
+// DefaultKubernetesResources returns the default implementation of the DefaultKubernetesResourcesModifier
+func DefaultKubernetesResources() tfsdk.AttributePlanModifier {
+	return DefaultKubernetesResourcesModifier{}
+}
+
+// DefaultKubernetesResourcesModifier implements the tfsdk.AttributePlanModifier interface. It accounts
+// for default values applied by CheckAndSetDefaults that would otherwise create inconsistent states
+type DefaultKubernetesResourcesModifier struct{}
+
+// Description of the KubernetesResources plan modifier
+func (d DefaultKubernetesResourcesModifier) Description(_ context.Context) string {
+	return "This modifier re-renders the role.spec.allow.kubernetes_resources from the user provided config instead of using the state. " +
+		DefaultModiferDescription
+}
+
+// MarkdownDescription of the KubernetesResources plan modifier
+func (d DefaultKubernetesResourcesModifier) MarkdownDescription(_ context.Context) string {
+	return "This modifier re-renders the role.spec.allow.kubernetes_resources from the user provided config instead of using the state. " +
+		DefaultModiferDescription
+}
+
+// Modify the terraform plan to account for defaults applied to KubernetesResources by CheckAndSetDefaults
+func (d DefaultKubernetesResourcesModifier) Modify(ctx context.Context, req tfsdk.ModifyAttributePlanRequest, resp *tfsdk.ModifyAttributePlanResponse) {
+	var config tftypes.Object
+	diags := req.Config.Get(ctx, &config)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to get config.")
+		return
+	}
+
+	role := &apitypes.RoleV6{}
+	diags = CopyRoleV6FromTerraform(ctx, config, role)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to create a role from the config.")
+		return
+	}
+
+	err := role.CheckAndSetDefaults()
+	if err != nil {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, fmt.Sprintf("Failed to set the role defaults: %s", err))
+		return
+	}
+
+	diags = CopyRoleV6ToTerraform(ctx, role, &config)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to convert back the role into a TF object.")
+		return
+	}
+
+	specRaw, ok := config.Attrs["spec"]
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to get 'spec' from TF object.")
+		return
+	}
+	spec, ok := specRaw.(tftypes.Object)
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to cast 'spec' as a TF object.")
+		return
+	}
+
+	allowRaw, ok := spec.Attrs["allow"]
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to cast 'allow' as a TF object.")
+		return
+	}
+
+	allow, ok := allowRaw.(tftypes.Object)
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to cast 'allow' as a TF object.")
+		return
+	}
+
+	kubernetesResourcesRaw, ok := allow.Attrs["kubernetes_resources"]
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to cast 'kubernetes_resources' as a TF list.")
+		return
+	}
+
+	kubernetesResources, ok := kubernetesResourcesRaw.(tftypes.List)
+	if !ok {
+		resp.Diagnostics.AddError(DefaultKubernetesResourcesModifierErrSummary, "Failed to cast 'kubernetes_resources' as a TF list.")
+		return
+	}
+	kubernetesResources.Null = false
+	resp.AttributePlan = kubernetesResources
+}