Fix TF role preset rule addition logic (#51320)

gravitational · Jan 22, 2025 · 05c917c · 05c917c
1 parent 44e86a8
commit 05c917c
Show file tree

Hide file tree

Showing 2 changed files with 140 additions and 32 deletions.
diff --git a/lib/services/presets.go b/lib/services/presets.go
@@ -637,35 +637,32 @@ func NewPresetTerraformProviderRole() types.Role {
 				WindowsDesktopLabels: map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
 				// Every resource currently supported by the Terraform provider.
 				Rules: []types.Rule{
-					{
-						Resources: []string{
-							types.KindAccessList,
-							types.KindApp,
-							types.KindClusterAuthPreference,
-							types.KindClusterMaintenanceConfig,
-							types.KindClusterNetworkingConfig,
-							types.KindDatabase,
-							types.KindDynamicWindowsDesktop,
-							types.KindDevice,
-							types.KindGithub,
-							types.KindLoginRule,
-							types.KindNode,
-							types.KindOIDC,
-							types.KindOktaImportRule,
-							types.KindRole,
-							types.KindSAML,
-							types.KindSessionRecordingConfig,
-							types.KindToken,
-							types.KindTrustedCluster,
-							types.KindUser,
-							types.KindBot,
-							types.KindInstaller,
-							types.KindAccessMonitoringRule,
-							types.KindStaticHostUser,
-							types.KindWorkloadIdentity,
-						},
-						Verbs: RW(),
-					},
+					// You must add new resources as separate rules for the
+					// default rule addition logic to work properly.
+					types.NewRule(types.KindAccessList, RW()),
+					types.NewRule(types.KindApp, RW()),
+					types.NewRule(types.KindClusterAuthPreference, RW()),
+					types.NewRule(types.KindClusterMaintenanceConfig, RW()),
+					types.NewRule(types.KindClusterNetworkingConfig, RW()),
+					types.NewRule(types.KindDatabase, RW()),
+					types.NewRule(types.KindDevice, RW()),
+					types.NewRule(types.KindGithub, RW()),
+					types.NewRule(types.KindLoginRule, RW()),
+					types.NewRule(types.KindNode, RW()),
+					types.NewRule(types.KindOIDC, RW()),
+					types.NewRule(types.KindOktaImportRule, RW()),
+					types.NewRule(types.KindRole, RW()),
+					types.NewRule(types.KindSAML, RW()),
+					types.NewRule(types.KindSessionRecordingConfig, RW()),
+					types.NewRule(types.KindToken, RW()),
+					types.NewRule(types.KindTrustedCluster, RW()),
+					types.NewRule(types.KindUser, RW()),
+					types.NewRule(types.KindBot, RW()),
+					types.NewRule(types.KindInstaller, RW()),
+					types.NewRule(types.KindAccessMonitoringRule, RW()),
+					types.NewRule(types.KindDynamicWindowsDesktop, RW()),
+					types.NewRule(types.KindStaticHostUser, RW()),
+					types.NewRule(types.KindWorkloadIdentity, RW()),
 				},
 			},
 		},
@@ -701,9 +698,10 @@ func bootstrapRoleMetadataLabels() map[string]map[string]string {
 }
 
 var defaultAllowRulesMap = map[string][]types.Rule{
-	teleport.PresetAuditorRoleName: NewPresetAuditorRole().GetRules(types.Allow),
-	teleport.PresetEditorRoleName:  NewPresetEditorRole().GetRules(types.Allow),
-	teleport.PresetAccessRoleName:  NewPresetAccessRole().GetRules(types.Allow),
+	teleport.PresetAuditorRoleName:           NewPresetAuditorRole().GetRules(types.Allow),
+	teleport.PresetEditorRoleName:            NewPresetEditorRole().GetRules(types.Allow),
+	teleport.PresetAccessRoleName:            NewPresetAccessRole().GetRules(types.Allow),
+	teleport.PresetTerraformProviderRoleName: NewPresetTerraformProviderRole().GetRules(types.Allow),
 }
 
 // defaultAllowRules has the Allow rules that should be set as default when

diff --git a/lib/services/presets_test.go b/lib/services/presets_test.go
@@ -29,6 +29,7 @@ import (
 	"github.com/gravitational/teleport/api/constants"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/types"
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/lib/modules"
 )
 
@@ -629,6 +630,115 @@ func TestAddRoleDefaults(t *testing.T) {
 				},
 			},
 		},
+		{
+			// This test is here to validate that we properly fix a bug previously introduced in the TF role preset.
+			// All the new resources got added into the same rule, but the preset defaults system only supports adding
+			// new rules, not editing existing ones. The resources got removed from the main rule and put into
+			// smaller individual rules.
+			name: "terraform provider (bugfix of the missing resources)",
+			role: &types.RoleV6{
+				Kind:    types.KindRole,
+				Version: types.V7,
+				Metadata: types.Metadata{
+					Name:        teleport.PresetTerraformProviderRoleName,
+					Namespace:   apidefaults.Namespace,
+					Description: "Default Terraform provider role",
+					Labels: map[string]string{
+						types.TeleportInternalResourceType: types.PresetResource,
+					},
+				},
+				Spec: types.RoleSpecV6{
+					Allow: types.RoleConditions{
+						AppLabels:            map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						DatabaseLabels:       map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						NodeLabels:           map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						WindowsDesktopLabels: map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						Rules: []types.Rule{
+							{
+								Resources: []string{
+									types.KindAccessList,
+									types.KindApp,
+									types.KindClusterAuthPreference,
+									types.KindClusterMaintenanceConfig,
+									types.KindClusterNetworkingConfig,
+									types.KindDatabase,
+									types.KindDevice,
+									types.KindGithub,
+									types.KindLoginRule,
+									types.KindNode,
+									types.KindOIDC,
+									types.KindOktaImportRule,
+									types.KindRole,
+									types.KindSAML,
+									types.KindSessionRecordingConfig,
+									types.KindToken,
+									types.KindTrustedCluster,
+									types.KindUser,
+									// Some of the new resources got introduced, but not all
+									types.KindBot,
+									types.KindInstaller,
+								},
+								Verbs: RW(),
+							},
+						},
+					},
+				},
+			},
+			expectedErr: require.NoError,
+			expected: &types.RoleV6{
+				Kind:    types.KindRole,
+				Version: types.V7,
+				Metadata: types.Metadata{
+					Name:        teleport.PresetTerraformProviderRoleName,
+					Namespace:   apidefaults.Namespace,
+					Description: "Default Terraform provider role",
+					Labels: map[string]string{
+						types.TeleportInternalResourceType: types.PresetResource,
+					},
+				},
+				Spec: types.RoleSpecV6{
+					Allow: types.RoleConditions{
+						AppLabels:            map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						DatabaseLabels:       map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						NodeLabels:           map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						WindowsDesktopLabels: map[string]apiutils.Strings{types.Wildcard: []string{types.Wildcard}},
+						Rules: []types.Rule{
+							{
+								Resources: []string{
+									types.KindAccessList,
+									types.KindApp,
+									types.KindClusterAuthPreference,
+									types.KindClusterMaintenanceConfig,
+									types.KindClusterNetworkingConfig,
+									types.KindDatabase,
+									types.KindDevice,
+									types.KindGithub,
+									types.KindLoginRule,
+									types.KindNode,
+									types.KindOIDC,
+									types.KindOktaImportRule,
+									types.KindRole,
+									types.KindSAML,
+									types.KindSessionRecordingConfig,
+									types.KindToken,
+									types.KindTrustedCluster,
+									types.KindUser,
+									// The resources that already got into the main rule are still present.
+									types.KindBot,
+									types.KindInstaller,
+								},
+								Verbs: RW(),
+							},
+							// The missing resources got added as individual rules
+							types.NewRule(types.KindAccessMonitoringRule, RW()),
+							types.NewRule(types.KindDynamicWindowsDesktop, RW()),
+							types.NewRule(types.KindStaticHostUser, RW()),
+							types.NewRule(types.KindWorkloadIdentity, RW()),
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {