Fix Terraform provider (#69435)

- add scoped bot support
- ignore scoped role revision
- enable and fix scoped tests

integrations/terraform/provider/internal/legacy/resource_teleport_bot.go

@@ -33,9 +33,11 @@ import (
 	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
 	machineidv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1"
 	apitypes "github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/scopes"
 	"github.com/gravitational/teleport/lib/utils/slices"
 
 	"github.com/gravitational/teleport/integrations/terraform/provider/internal/tfdiag"
+	"github.com/gravitational/teleport/integrations/terraform/provider/internal/tfdriver"
 	"github.com/gravitational/teleport/integrations/terraform/tfschema"
 )
 
@@ -288,6 +290,7 @@ func (r resourceTeleportBot) Read(ctx context.Context, req tfsdk.ReadResourceReq
 
 	bot, err := r.p.Client().BotServiceClient().GetBot(ctx, &machineidv1.GetBotRequest{
 		BotName: state.GetName(),
+		Scope:   state.GetScope(),
 	})
 	switch {
 	case trace.IsNotFound(err):
@@ -350,6 +353,8 @@ func (r resourceTeleportBot) botFromProto(ctx context.Context, bot *machineidv1.
 		return !ok || attr.IsNull()
 	}
 
+	sqn := scopes.QualifiedName{Name: bot.GetMetadata().GetName(), Scope: bot.GetScope()}
+
 	result := Bot{
 		// User-provided attributes. Will be marked as null based on whether the
 		// user provided legacy or RFD 153-style attributes.
@@ -361,7 +366,7 @@ func (r resourceTeleportBot) botFromProto(ctx context.Context, bot *machineidv1.
 		Name: types.String{},
 
 		// Computed attributes.
-		ID:      stringValue(bot.GetMetadata().GetName()),
+		ID:      stringValue(sqn.String()),
 		Kind:    stringValue(bot.GetKind()),
 		SubKind: stringValue(bot.GetSubKind()),
 		Version: stringValue(bot.GetVersion()),
@@ -523,7 +528,10 @@ func (r resourceTeleportBot) Delete(ctx context.Context, req tfsdk.DeleteResourc
 		return
 	}
 	_, err := r.p.Client().BotServiceClient().
-		DeleteBot(ctx, &machineidv1.DeleteBotRequest{BotName: state.GetName()})
+		DeleteBot(ctx, &machineidv1.DeleteBotRequest{
+			BotName: state.GetName(),
+			Scope:   state.GetScope(),
+		})
 	if err != nil {
 		resp.Diagnostics.Append(tfdiag.DiagFromWrappedErr("Error deleting Bot", trace.Wrap(err), "bot"))
 		return
@@ -533,8 +541,17 @@ func (r resourceTeleportBot) Delete(ctx context.Context, req tfsdk.DeleteResourc
 }
 
 func (r resourceTeleportBot) ImportState(ctx context.Context, req tfsdk.ImportResourceStateRequest, rsp *tfsdk.ImportResourceStateResponse) {
+	sqn, err := tfdriver.NewPossiblyUnscopedScopeQualifiedNameIdentifier(req.ID)
+	if err != nil {
+		rsp.Diagnostics.AddError("Error parsing bot ID", err.Error())
+		return
+	}
+
 	bot, err := r.p.Client().BotServiceClient().
-		GetBot(ctx, &machineidv1.GetBotRequest{BotName: req.ID})
+		GetBot(ctx, &machineidv1.GetBotRequest{
+			BotName: sqn.Name,
+			Scope:   sqn.Scope,
+		})
 	if err != nil {
 		rsp.Diagnostics.Append(tfdiag.DiagFromWrappedErr("Error reading Bot", trace.Wrap(err), "bot"))
 		return

integrations/terraform/testlib/bot_test.go

@@ -31,6 +31,7 @@ import (
 	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
 	machineidv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/scopes"
 )
 
 func (s *TerraformSuiteOSS) TestBot() {
@@ -273,17 +274,23 @@ func (s *TerraformSuiteOSSScopedResources) TestBotWithScope() {
 	ctx := t.Context()
 
 	checkResourcesDestroyed := func(state *terraform.State) error {
+		const tokenResourceName = "bot-test-scoped"
 		var errs []error
-		if _, err := s.client.GetToken(ctx, "bot-test-scoped"); err != nil {
+		if _, err := s.client.GetToken(ctx, tokenResourceName); err == nil {
+			errs = append(errs, fmt.Errorf("token %q still exists", tokenResourceName))
+		} else {
 			if !trace.IsNotFound(err) {
-				errs = append(errs, err)
+				errs = append(errs, trace.Wrap(err, "failed to get token"))
 			}
 		}
 
-		if _, err := s.client.GetUser(ctx, "bot-test-scoped-bot", false); err != nil {
-			if !trace.IsNotFound(err) {
-				errs = append(errs, err)
-			}
+		const botResourceName = "test-scoped-bot"
+		if _, err := s.client.BotServiceClient().GetBot(ctx,
+			&machineidv1.GetBotRequest{BotName: botResourceName, Scope: "/test-scope"},
+		); err == nil {
+			errs = append(errs, fmt.Errorf("bot %q still exists", botResourceName))
+		} else if !trace.IsNotFound(err) {
+			errs = append(errs, trace.Wrap(err, "failed to get bot"))
 		}
 
 		return trace.NewAggregate(errs...)
@@ -309,7 +316,7 @@ func (s *TerraformSuiteOSSScopedResources) TestBotWithScope() {
 					resource.TestCheckResourceAttr(assignmentName, "metadata.name", "test-bot-assignment"),
 					resource.TestCheckResourceAttr(assignmentName, "scope", "/test-scope"),
 					resource.TestCheckResourceAttr(assignmentName, "spec.bot", "/test-scope::test-scoped-bot"),
-					resource.TestCheckResourceAttr(assignmentName, "spec.assignments.0.role", "scoped-operator"),
+					resource.TestCheckResourceAttr(assignmentName, "spec.assignments.0.role", "/test-scope::scoped-operator"),
 					resource.TestCheckResourceAttr(assignmentName, "spec.assignments.0.scope", "/test-scope"),
 				),
 			},
@@ -349,3 +356,55 @@ func (s *TerraformSuiteOSSScopedResources) TestBotWithScope() {
 		},
 	})
 }
+
+func (s *TerraformSuiteOSSScopedResources) TestImportScopedBot() {
+	t := s.T()
+	ctx := t.Context()
+
+	r := "teleport_bot"
+	id := "test_import"
+	name := r + "." + id
+	const testScope = "/staging"
+
+	bot := &machineidv1.Bot{
+		Kind:    types.KindBot,
+		Version: types.V1,
+		Metadata: &headerv1.Metadata{
+			Name: id,
+		},
+		Spec:  &machineidv1.BotSpec{},
+		Scope: testScope,
+	}
+	bot, err := s.client.BotServiceClient().
+		CreateBot(ctx, &machineidv1.CreateBotRequest{Bot: bot})
+	require.NoError(t, err)
+
+	require.Eventually(t, func() bool {
+		_, err := s.client.BotServiceClient().
+			GetBot(ctx, &machineidv1.GetBotRequest{
+				BotName: bot.Metadata.Name,
+				Scope:   testScope,
+			})
+		return err == nil
+	}, 5*time.Second, time.Second)
+
+	sqn := scopes.QualifiedName{Name: id, Scope: testScope}
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: s.terraformProviders,
+		IsUnitTest:               true,
+		Steps: []resource.TestStep{
+			{
+				Config:        fmt.Sprintf("%s\nresource %q %q { }", s.terraformConfig, r, id),
+				ResourceName:  name,
+				ImportState:   true,
+				ImportStateId: sqn.String(),
+				ImportStateCheck: func(state []*terraform.InstanceState) error {
+					assert.Equal(t, types.KindBot, state[0].Attributes["kind"])
+					assert.Equal(t, id, state[0].Attributes["metadata.name"])
+					return nil
+				},
+			},
+		},
+	})
+}

integrations/terraform/testlib/fixtures/bot_scoped_0_create.tf

@@ -43,7 +43,7 @@ resource "teleport_scoped_role_assignment" "bot_assignment" {
   spec = {
     bot = "${teleport_bot.test_scoped.scope}::${teleport_bot.test_scoped.metadata.name}"
     assignments = [{
-      role  = teleport_scoped_role.scoped_operator.metadata.name
+      role  = "${teleport_scoped_role.scoped_operator.scope}::${teleport_scoped_role.scoped_operator.metadata.name}"
       scope = local.scope_path
     }]
   }

integrations/terraform/testlib/fixtures/bot_scoped_1_update.tf

@@ -47,8 +47,8 @@ resource "teleport_scoped_role_assignment" "bot_assignment" {
   spec = {
     bot = "${teleport_bot.test_scoped.scope}::${teleport_bot.test_scoped.metadata.name}"
     assignments = [{
-      role  = teleport_scoped_role.scoped_operator.metadata.name
+      role  = "${teleport_scoped_role.scoped_operator.scope}::${teleport_scoped_role.scoped_operator.metadata.name}"
       scope = local.scope_path
     }]
   }
-}
+}

integrations/terraform/testlib/fixtures/bot_scoped_2_change_scope.tf

@@ -2,7 +2,6 @@ locals {
   bot_name_scoped = "test-scoped-bot"
   scope_path      = "/different-scope"
 }
-
 resource "teleport_scoped_role" "scoped_operator" {
   version = "v1"
   metadata = {
@@ -47,8 +46,8 @@ resource "teleport_scoped_role_assignment" "bot_assignment" {
   spec = {
     bot = "${teleport_bot.test_scoped.scope}::${teleport_bot.test_scoped.metadata.name}"
     assignments = [{
-      role  = teleport_scoped_role.scoped_operator.metadata.name
+      role  = "${teleport_scoped_role.scoped_operator.scope}::${teleport_scoped_role.scoped_operator.metadata.name}"
       scope = local.scope_path
     }]
   }
-}
+}

integrations/terraform/testlib/fixtures/scoped_role_assignment_0_create.tf

@@ -1,15 +1,19 @@
+locals {
+  scope = "/staging"
+}
+
 resource "teleport_scoped_role_assignment" "test" {
   version = "v1"
   metadata = {
     name = "test-scoped-role-assignment"
   }
-  scope    = "/staging"
+  scope    = local.scope
   sub_kind = "dynamic"
   spec = {
     user = "testuser"
     assignments = [{
-      role  = "test-scoped-role"
-      scope = "/staging/aa"
+      role  = "${local.scope}::test-scoped-role"
+      scope = "${local.scope}/aa"
     }]
   }
 }

integrations/terraform/testlib/fixtures/scoped_role_assignment_1_update.tf

@@ -1,15 +1,19 @@
+locals {
+  scope = "/staging"
+}
+
 resource "teleport_scoped_role_assignment" "test" {
   version = "v1"
   metadata = {
     name = "test-scoped-role-assignment"
   }
-  scope    = "/staging"
+  scope    = local.scope
   sub_kind = "dynamic"
   spec = {
     user = "testuser"
     assignments = [{
-      role  = "test-scoped-role"
-      scope = "/staging/aaaa"
+      role  = "${local.scope}::test-scoped-role"
+      scope = "${local.scope}/aaaa"
     }]
   }
 }

integrations/terraform/testlib/scoped_role_assignment_test.go

@@ -29,6 +29,7 @@ import (
 	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
 	accessv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/scopes/access/v1"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/scopes"
 	"github.com/gravitational/teleport/lib/scopes/access"
 )
 
@@ -41,6 +42,7 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRoleAssignment() {
 
 		_, err := accessClient.GetScopedRoleAssignment(ctx, &accessv1.GetScopedRoleAssignmentRequest{
 			Name:    "test-scoped-role-assignment",
+			Scope:   "/staging",
 			SubKind: access.SubKindDynamic,
 		})
 		if !trace.IsNotFound(err) {
@@ -63,7 +65,7 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRoleAssignment() {
 					resource.TestCheckResourceAttr(name, "sub_kind", access.SubKindDynamic),
 					resource.TestCheckResourceAttr(name, "scope", "/staging"),
 					resource.TestCheckResourceAttr(name, "spec.user", "testuser"),
-					resource.TestCheckResourceAttr(name, "spec.assignments.0.role", "test-scoped-role"),
+					resource.TestCheckResourceAttr(name, "spec.assignments.0.role", "/staging::test-scoped-role"),
 					resource.TestCheckResourceAttr(name, "spec.assignments.0.scope", "/staging/aa"),
 				),
 			},
@@ -71,7 +73,7 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRoleAssignment() {
 				Config: s.getFixture("scoped_role_assignment_1_update.tf"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(name, "scope", "/staging"),
-					resource.TestCheckResourceAttr(name, "spec.assignments.0.role", "test-scoped-role"),
+					resource.TestCheckResourceAttr(name, "spec.assignments.0.role", "/staging::test-scoped-role"),
 					resource.TestCheckResourceAttr(name, "spec.assignments.0.scope", "/staging/aaaa"),
 				),
 			},
@@ -91,6 +93,7 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRoleAssignment() {
 	r := "teleport_scoped_role_assignment"
 	id := "test-import-sra"
 	name := r + "." + id
+	const testScope = "/staging"
 
 	assignment := &accessv1.ScopedRoleAssignment{
 		Kind:    access.KindScopedRoleAssignment,
@@ -99,13 +102,13 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRoleAssignment() {
 		Metadata: &headerv1.Metadata{
 			Name: id,
 		},
-		Scope: "/staging",
+		Scope: testScope,
 		Spec: &accessv1.ScopedRoleAssignmentSpec{
 			User: "testuser",
 			Assignments: []*accessv1.Assignment{
 				{
-					Role:  "test-scoped-role",
-					Scope: "/staging/aa",
+					Role:  testScope + "::test-scoped-role",
+					Scope: testScope + "/aa",
 				},
 			},
 		},
@@ -119,11 +122,13 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRoleAssignment() {
 	require.EventuallyWithT(t, func(t *assert.CollectT) {
 		_, err := accessClient.GetScopedRoleAssignment(ctx, &accessv1.GetScopedRoleAssignmentRequest{
 			Name:    id,
+			Scope:   testScope,
 			SubKind: access.SubKindDynamic,
 		})
 		require.NoError(t, err)
 	}, 5*time.Second, time.Second)
 
+	sqn := scopes.QualifiedName{Scope: testScope, Name: id}
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: s.terraformProviders,
 		IsUnitTest:               true,
@@ -132,7 +137,7 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRoleAssignment() {
 				Config:        fmt.Sprintf("%s\nresource %q %q { }", s.terraformConfig, r, id),
 				ResourceName:  name,
 				ImportState:   true,
-				ImportStateId: id,
+				ImportStateId: sqn.String(),
 				ImportStateCheck: func(state []*terraform.InstanceState) error {
 					require.Equal(t, access.KindScopedRoleAssignment, state[0].Attributes["kind"])
 					require.Equal(t, "/staging", state[0].Attributes["scope"])

integrations/terraform/testlib/scoped_role_test.go

@@ -29,6 +29,7 @@ import (
 	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
 	accessv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/scopes/access/v1"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/scopes"
 	"github.com/gravitational/teleport/lib/scopes/access"
 )
 
@@ -36,10 +37,12 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRole() {
 	t := s.T()
 	ctx := t.Context()
 
+	const testScope = "/staging"
 	checkDestroyed := func(state *terraform.State) error {
 		accessClient := s.client.ScopedAccessServiceClient()
 		_, err := accessClient.GetScopedRole(ctx, &accessv1.GetScopedRoleRequest{
-			Name: "test-scoped-role",
+			Name:  "test-scoped-role",
+			Scope: testScope,
 		})
 		if !trace.IsNotFound(err) {
 			return trace.Errorf("expected not found, actual: %v", err)
@@ -58,8 +61,8 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRole() {
 				Config: s.getFixture("scoped_role_0_create.tf"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(name, "kind", access.KindScopedRole),
-					resource.TestCheckResourceAttr(name, "scope", "/staging"),
-					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.0", "/staging/aa"),
+					resource.TestCheckResourceAttr(name, "scope", testScope),
+					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.0", testScope+"/aa"),
 					resource.TestCheckResourceAttr(name, "spec.rules.0.resources.0", "scoped_token"),
 					resource.TestCheckResourceAttr(name, "spec.rules.0.verbs.0", "read"),
 					resource.TestCheckResourceAttr(name, "spec.rules.0.verbs.1", "list"),
@@ -73,8 +76,8 @@ func (s *TerraformSuiteOSSScopedResources) TestScopedRole() {
 				Config: s.getFixture("scoped_role_1_update.tf"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(name, "metadata.labels.env", "staging"),
-					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.0", "/staging/aa"),
-					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.1", "/staging/bb"),
+					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.0", testScope+"/aa"),
+					resource.TestCheckResourceAttr(name, "spec.assignable_scopes.1", testScope+"/bb"),
 					resource.TestCheckResourceAttr(name, "spec.rules.0.verbs.2", "create"),
 					resource.TestCheckResourceAttr(name, "spec.ssh.logins.0", "root"),
 				),
@@ -96,13 +99,15 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRole() {
 	id := "test_import_scoped_role"
 	name := r + "." + id
 
+	const testScope = "/staging"
+
 	role := &accessv1.ScopedRole{
 		Kind:    access.KindScopedRole,
 		Version: types.V1,
 		Metadata: &headerv1.Metadata{
 			Name: id,
 		},
-		Scope: "/staging",
+		Scope: testScope,
 		Spec: &accessv1.ScopedRoleSpec{
 			AssignableScopes: []string{"/staging/aa"},
 			Rules: []*accessv1.ScopedRule{
@@ -121,11 +126,14 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRole() {
 
 	require.EventuallyWithT(t, func(t *assert.CollectT) {
 		_, err := accessClient.GetScopedRole(ctx, &accessv1.GetScopedRoleRequest{
-			Name: id,
+			Name:  id,
+			Scope: testScope,
 		})
 		require.NoError(t, err)
 	}, 5*time.Second, time.Second)
 
+	sqn := scopes.QualifiedName{Scope: testScope, Name: id}
+
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: s.terraformProviders,
 		IsUnitTest:               true,
@@ -134,7 +142,7 @@ func (s *TerraformSuiteOSSScopedResources) TestImportScopedRole() {
 				Config:        fmt.Sprintf("%s\nresource %q %q { }", s.terraformConfig, r, id),
 				ResourceName:  name,
 				ImportState:   true,
-				ImportStateId: id,
+				ImportStateId: sqn.String(),
 				ImportStateCheck: func(state []*terraform.InstanceState) error {
 					require.Equal(t, access.KindScopedRole, state[0].Attributes["kind"])
 					require.Equal(t, "/staging", state[0].Attributes["scope"])

integrations/terraform/testlib/scoped_token_test.go

@@ -132,15 +132,17 @@ func (s *TerraformSuiteOSS) TestImportScopedToken() {
 	id := "test_import_scoped_token"
 	name := r + "." + id
 
+	const testScope = "/staging"
+
 	token := &joiningv1.ScopedToken{
 		Kind:    types.KindScopedToken,
 		Version: types.V1,
 		Metadata: &headerv1.Metadata{
 			Name: id,
 		},
-		Scope: "/staging",
+		Scope: testScope,
 		Spec: &joiningv1.ScopedTokenSpec{
-			AssignedScope: "/staging/nodes",
+			AssignedScope: testScope + "/nodes",
 			JoinMethod:    "token",
 			Roles:         []string{"Node"},
 			UsageMode:     "unlimited",
@@ -153,12 +155,14 @@ func (s *TerraformSuiteOSS) TestImportScopedToken() {
 	require.EventuallyWithT(t, func(t *assert.CollectT) {
 		_, err := s.client.GetScopedToken(ctx, joiningv1.GetScopedTokenRequest_builder{
 			Name:       id,
-			Scope:      "/staging",
+			Scope:      testScope,
 			WithSecret: true,
 		}.Build())
 		require.NoError(t, err)
 	}, 5*time.Second, time.Second)
 
+	sqn := scopes.QualifiedName{Scope: testScope, Name: id}
+
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: s.terraformProviders,
 		IsUnitTest:               true,
@@ -167,11 +171,11 @@ func (s *TerraformSuiteOSS) TestImportScopedToken() {
 				Config:        fmt.Sprintf("%s\nresource %q %q { }", s.terraformConfig, r, id),
 				ResourceName:  name,
 				ImportState:   true,
-				ImportStateId: scopes.QualifiedName{Scope: token.GetScope(), Name: token.GetMetadata().GetName()}.String(),
+				ImportStateId: sqn.String(),
 				ImportStateCheck: func(state []*terraform.InstanceState) error {
 					require.Equal(t, types.KindScopedToken, state[0].Attributes["kind"])
-					require.Equal(t, "/staging", state[0].Attributes["scope"])
-					require.Equal(t, "/staging/nodes", state[0].Attributes["spec.assigned_scope"])
+					require.Equal(t, testScope, state[0].Attributes["scope"])
+					require.Equal(t, testScope+"/nodes", state[0].Attributes["spec.assigned_scope"])
 					return nil
 				},
 			},

integrations/terraform/testlib/terraform_oss_test.go

@@ -35,7 +35,6 @@ func TestTerraformOSS(t *testing.T) {
 }
 
 func TestTerraformOSSScopedResources(t *testing.T) {
-	t.Skip("scope namespaced resources are temporarily non-functional until we can update terraform to be compatible with namespacing")
 	suite.Run(t, &TerraformSuiteOSSScopedResources{
 		TerraformBaseSuite: TerraformBaseSuite{
 			AuthHelper: &integration.MinimalAuthHelper{