Added unique scope to all terraform items (#2233)

This will be replaced with the real GUN if it gets mapped, but if not
we'll get a truly unique path

GitOrigin-RevId: 0eaaa13a87dd63ac7240d9b5de1f5a64bd4fe2c7

Author: dylanratcliffe

cmd/changes_submit_plan.go

@@ -99,6 +99,18 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	lf := log.Fields{}
+
+	// Detect the repository URL if it wasn't provided
+	repoUrl := viper.GetString("repo")
+	if repoUrl == "" {
+		repoUrl, err = DetectRepoURL(AllDetectors)
+		if err != nil {
+			log.WithContext(ctx).WithError(err).WithFields(lf).Debug("Failed to detect repository URL. Use the --repo flag to specify it manually if you require it")
+		}
+	}
+	scope := tfutils.RepoToScope(repoUrl)
+
 	fileWord := "file"
 	if len(args) > 1 {
 		fileWord = "files"
@@ -108,10 +120,9 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 
 	plannedChanges := make([]*sdp.MappedItemDiff, 0)
 
-	lf := log.Fields{}
 	for _, f := range args {
 		lf["file"] = f
-		result, err := tfutils.MappedItemDiffsFromPlanFile(ctx, f, lf)
+		result, err := tfutils.MappedItemDiffsFromPlanFile(ctx, f, scope, lf)
 		if err != nil {
 			return loggedError{
 				err:     err,
@@ -136,14 +147,7 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 	title := changeTitle(viper.GetString("title"))
 	tfPlanOutput := tryLoadText(ctx, viper.GetString("terraform-plan-output"))
 	codeChangesOutput := tryLoadText(ctx, viper.GetString("code-changes-diff"))
-	// Detect the repository URL if it wasn't provided
-	repoUrl := viper.GetString("repo")
-	if repoUrl == "" {
-		repoUrl, err = DetectRepoURL(AllDetectors)
-		if err != nil {
-			log.WithContext(ctx).WithError(err).WithFields(lf).Debug("Failed to detect repository URL. Use the --repo flag to specify it manually if you require it")
-		}
-	}
+
 	enrichedTags, err := parseTagsArgument()
 	if err != nil {
 		return loggedError{

cmd/repo.go

@@ -16,6 +16,7 @@ var AllDetectors = []RepoDetector{
 	&RepoDetectorCircleCI{},
 	&RepoDetectorAzureDevOps{},
 	&RepoDetectorSpacelift{},
+	&RepoDetectorScalr{},
 	&RepoDetectorGitConfig{},
 }
 
@@ -250,3 +251,29 @@ func (d *RepoDetectorGitConfig) DetectRepoURL(envVars map[string]string) (string
 
 	return "", fmt.Errorf("could not find remote URL in %v", gitConfigPath)
 }
+
+type RepoDetectorScalr struct{}
+
+func (d *RepoDetectorScalr) RequiredEnvVars() []string {
+	return []string{"SCALR_WORKSPACE_NAME", "SCALR_ENVIRONMENT_NAME"}
+}
+
+func (d *RepoDetectorScalr) DetectRepoURL(envVars map[string]string) (string, error) {
+	workspaceName, ok := envVars["SCALR_WORKSPACE_NAME"]
+	if !ok {
+		return "", errors.New("SCALR_WORKSPACE_NAME not set")
+	}
+
+	environmentName, ok := envVars["SCALR_ENVIRONMENT_NAME"]
+	if !ok {
+		return "", errors.New("SCALR_ENVIRONMENT_NAME not set")
+	}
+
+	// A full Scalr URL can be constructed using
+	// "https://$SCALR_HOSTNAME/v2/e/$SCALR_ENVIRONMENT_ID/workspaces/$SCALR_WORKSPACE_ID".
+	// The problem with this is that the environment and workspace IDs are
+	// computer generated and people aren't likely to understand what they mean.
+	// Therefore we are going to go with custom URL scheme to make sure that the
+	// URL is readable
+	return fmt.Sprintf("scalr://%s/%s", environmentName, workspaceName), nil
+}

cmd/repo_test.go

@@ -489,3 +489,137 @@ func TestRepoDetectorGitConfig(t *testing.T) {
 		}
 	})
 }
+
+func TestRepoDetectorScalr(t *testing.T) {
+	t.Parallel()
+
+	t.Run("with valid SCALR_WORKSPACE_NAME and SCALR_ENVIRONMENT_NAME", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{
+			"SCALR_WORKSPACE_NAME":   "my-workspace",
+			"SCALR_ENVIRONMENT_NAME": "production",
+		}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		expectedRepoURL := "scalr://production/my-workspace"
+		if repoURL != expectedRepoURL {
+			t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
+		}
+	})
+
+	t.Run("with missing SCALR_WORKSPACE_NAME", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{
+			"SCALR_ENVIRONMENT_NAME": "production",
+		}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+		if repoURL != "" {
+			t.Fatalf("expected empty repoURL, got %q", repoURL)
+		}
+
+		expectedError := "SCALR_WORKSPACE_NAME not set"
+		if err.Error() != expectedError {
+			t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
+		}
+	})
+
+	t.Run("with missing SCALR_ENVIRONMENT_NAME", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{
+			"SCALR_WORKSPACE_NAME": "my-workspace",
+		}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+		if repoURL != "" {
+			t.Fatalf("expected empty repoURL, got %q", repoURL)
+		}
+
+		expectedError := "SCALR_ENVIRONMENT_NAME not set"
+		if err.Error() != expectedError {
+			t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
+		}
+	})
+
+	t.Run("with both variables missing", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+		if repoURL != "" {
+			t.Fatalf("expected empty repoURL, got %q", repoURL)
+		}
+
+		expectedError := "SCALR_WORKSPACE_NAME not set"
+		if err.Error() != expectedError {
+			t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
+		}
+	})
+
+	t.Run("with empty values", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{
+			"SCALR_WORKSPACE_NAME":   "",
+			"SCALR_ENVIRONMENT_NAME": "",
+		}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		expectedRepoURL := "scalr:///"
+		if repoURL != expectedRepoURL {
+			t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
+		}
+	})
+
+	t.Run("with special characters", func(t *testing.T) {
+		t.Parallel()
+
+		envVars := map[string]string{
+			"SCALR_WORKSPACE_NAME":   "my-workspace-with-dashes_and_underscores",
+			"SCALR_ENVIRONMENT_NAME": "prod-env_123",
+		}
+
+		detector := &RepoDetectorScalr{}
+
+		repoURL, err := detector.DetectRepoURL(envVars)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		expectedRepoURL := "scalr://prod-env_123/my-workspace-with-dashes_and_underscores"
+		if repoURL != expectedRepoURL {
+			t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
+		}
+	})
+}

cmd/terraform_plan.go

@@ -127,6 +127,12 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
 
 	removingSecretsSpinner.Success()
 
+	// Detect the repository URL if it wasn't provided
+	repoUrl := viper.GetString("repo")
+	if repoUrl == "" {
+		repoUrl, _ = DetectRepoURL(AllDetectors)
+	}
+
 	///////////////////////////////////////////////////////////////////
 	// Extract changes from the plan and created mapped item diffs
 	///////////////////////////////////////////////////////////////////
@@ -135,8 +141,10 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
 	resourceExtractionResults := multi.NewWriter()
 	time.Sleep(200 * time.Millisecond) // give the UI a little time to update
 
+	scope := tfutils.RepoToScope(repoUrl)
+
 	// Map the terraform changes to Overmind queries
-	mappingResponse, err := tfutils.MappedItemDiffsFromPlan(ctx, planJson, planFile, log.Fields{})
+	mappingResponse, err := tfutils.MappedItemDiffsFromPlan(ctx, planJson, planFile, scope, log.Fields{})
 	if err != nil {
 		resourceExtractionSpinner.Fail(fmt.Sprintf("Removing secrets: %v", err))
 		return nil
@@ -230,11 +238,6 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
 
 	codeChangesOutput := tryLoadText(ctx, viper.GetString("code-changes-diff"))
 
-	// Detect the repository URL if it wasn't provided
-	repoUrl := viper.GetString("repo")
-	if repoUrl == "" {
-		repoUrl, _ = DetectRepoURL(AllDetectors)
-	}
 	enrichedTags, err := parseTagsArgument()
 	if err != nil {
 		uploadChangesSpinner.Fail(fmt.Sprintf("Uploading planned changes: failed to parse tags: %v", err))

tfutils/plan_mapper.go

@@ -106,15 +106,15 @@ func (r *PlanMappingResult) numStatus(status MapStatus) int {
 	return count
 }
 
-func MappedItemDiffsFromPlanFile(ctx context.Context, fileName string, lf log.Fields) (*PlanMappingResult, error) {
+func MappedItemDiffsFromPlanFile(ctx context.Context, fileName string, scope string, lf log.Fields) (*PlanMappingResult, error) {
 	// read results from `terraform show -json ${tfplan file}`
 	planJSON, err := os.ReadFile(fileName)
 	if err != nil {
 		log.WithContext(ctx).WithError(err).WithFields(lf).Error("Failed to read terraform plan")
 		return nil, err
 	}
 
-	return MappedItemDiffsFromPlan(ctx, planJSON, fileName, lf)
+	return MappedItemDiffsFromPlan(ctx, planJSON, fileName, scope, lf)
 }
 
 type TfMapData struct {
@@ -135,8 +135,9 @@ type TfMapData struct {
 // and current resource values. The function categorizes the mapped item
 // differences into supported and unsupported changes. Finally, it logs the
 // number of supported and unsupported changes and returns the mapped item
-// differences.
-func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName string, lf log.Fields) (*PlanMappingResult, error) {
+// differences. The `scope` determines the scope of the resources that will be
+// generated, not the queries. These will always have a scope of `*`
+func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName string, scope string, lf log.Fields) (*PlanMappingResult, error) {
 	// Create a span for this since we're going to be attaching events to it when things fail to map
 	span := trace.SpanFromContext(ctx)
 	defer span.End()
@@ -195,7 +196,7 @@ func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName stri
 			continue
 		}
 
-		itemDiff, err := itemDiffFromResourceChange(resourceChange)
+		itemDiff, err := itemDiffFromResourceChange(resourceChange, scope)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create item diff for resource change: %w", err)
 		}
@@ -440,9 +441,7 @@ func countSensitiveAttributes(attributes, sensitive any) int {
 }
 
 // Converts a ResourceChange form a terraform plan to an ItemDiff in SDP format.
-// These items will use the scope `terraform_plan` since we haven't mapped them
-// to an actual item in the infrastructure yet
-func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, error) {
+func itemDiffFromResourceChange(resourceChange ResourceChange, scope string) (*sdp.ItemDiff, error) {
 	status := sdp.ItemDiffStatus_ITEM_DIFF_STATUS_UNSPECIFIED
 
 	if slices.Equal(resourceChange.Change.Actions, []string{"no-op"}) || slices.Equal(resourceChange.Change.Actions, []string{"read"}) {
@@ -488,7 +487,7 @@ func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, e
 			Type:            resourceChange.Type,
 			UniqueAttribute: "terraform_name",
 			Attributes:      beforeAttributes,
-			Scope:           "terraform_plan",
+			Scope:           scope,
 		}
 
 		err = result.GetBefore().GetAttributes().Set("terraform_name", trimmedAddress)
@@ -509,7 +508,7 @@ func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, e
 			Type:            resourceChange.Type,
 			UniqueAttribute: "terraform_name",
 			Attributes:      afterAttributes,
-			Scope:           "terraform_plan",
+			Scope:           scope,
 		}
 
 		err = result.GetAfter().GetAttributes().Set("terraform_name", trimmedAddress)

tfutils/plan_mapper_test.go

@@ -15,7 +15,7 @@ import (
 )
 
 func TestWithStateFile(t *testing.T) {
-	_, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/state.json", log.Fields{})
+	_, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/state.json", "scope", log.Fields{})
 
 	if err == nil {
 		t.Error("Expected error when running with state file, got none")
@@ -48,7 +48,7 @@ func TestExtractProviderNameFromConfigKey(t *testing.T) {
 }
 
 func TestMappedItemDiffsFromPlan(t *testing.T) {
-	results, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/plan.json", log.Fields{})
+	results, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/plan.json", "scope", log.Fields{})
 	if err != nil {
 		t.Error(err)
 	}
@@ -120,8 +120,8 @@ func TestMappedItemDiffsFromPlan(t *testing.T) {
 	if nats_box_deployment.GetMappingQuery().GetScope() != "*" {
 		t.Errorf("Expected nats_box_deployment query scope to be '*', got '%v'", nats_box_deployment.GetMappingQuery().GetScope())
 	}
-	if nats_box_deployment.GetItem().GetBefore().GetScope() != "terraform_plan" {
-		t.Errorf("Expected nats_box_deployment before item scope to be 'terraform_plan', got '%v'", nats_box_deployment.GetItem().GetBefore().GetScope())
+	if nats_box_deployment.GetItem().GetBefore().GetScope() != "scope" {
+		t.Errorf("Expected nats_box_deployment before item scope to be 'scope', got '%v'", nats_box_deployment.GetItem().GetBefore().GetScope())
 	}
 	if nats_box_deployment.GetMappingQuery().GetType() != "Deployment" {
 		t.Errorf("Expected nats_box_deployment query type to be 'Deployment', got '%v'", nats_box_deployment.GetMappingQuery().GetType())
@@ -150,8 +150,8 @@ func TestMappedItemDiffsFromPlan(t *testing.T) {
 	if api_server_deployment.GetMappingQuery().GetScope() != "*" {
 		t.Errorf("Expected api_server_deployment query scope to be '*', got '%v'", api_server_deployment.GetMappingQuery().GetScope())
 	}
-	if api_server_deployment.GetItem().GetBefore().GetScope() != "terraform_plan" {
-		t.Errorf("Expected api_server_deployment before item scope to be 'terraform_plan', got '%v'", api_server_deployment.GetItem().GetBefore().GetScope())
+	if api_server_deployment.GetItem().GetBefore().GetScope() != "scope" {
+		t.Errorf("Expected api_server_deployment before item scope to be 'scope', got '%v'", api_server_deployment.GetItem().GetBefore().GetScope())
 	}
 	if api_server_deployment.GetMappingQuery().GetType() != "Deployment" {
 		t.Errorf("Expected api_server_deployment query type to be 'Deployment', got '%v'", api_server_deployment.GetMappingQuery().GetType())
@@ -180,8 +180,8 @@ func TestMappedItemDiffsFromPlan(t *testing.T) {
 	if aws_iam_policy.GetMappingQuery().GetScope() != "*" {
 		t.Errorf("Expected aws_iam_policy query scope to be '*', got '%v'", aws_iam_policy.GetMappingQuery().GetScope())
 	}
-	if aws_iam_policy.GetItem().GetBefore().GetScope() != "terraform_plan" {
-		t.Errorf("Expected aws_iam_policy before item scope to be 'terraform_plan', got '%v'", aws_iam_policy.GetItem().GetBefore().GetScope())
+	if aws_iam_policy.GetItem().GetBefore().GetScope() != "scope" {
+		t.Errorf("Expected aws_iam_policy before item scope to be 'scope', got '%v'", aws_iam_policy.GetItem().GetBefore().GetScope())
 	}
 	if aws_iam_policy.GetMappingQuery().GetType() != "iam-policy" {
 		t.Errorf("Expected aws_iam_policy query type to be 'iam-policy', got '%v'", aws_iam_policy.GetMappingQuery().GetType())