Skip to content

Commit 4357ab9

Browse files
dylanratcliffeactions-user
dylanratcliffe
authored and
actions-user
committed
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
1 parent da1ee07 commit 4357ab9

File tree

8 files changed

+324
-34
lines changed

8 files changed

+324
-34
lines changed

‎cmd/changes_submit_plan.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,18 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
9999
return err
100100
}
101101

102+
lf := log.Fields{}
103+
104+
// Detect the repository URL if it wasn't provided
105+
repoUrl := viper.GetString("repo")
106+
if repoUrl == "" {
107+
repoUrl, err = DetectRepoURL(AllDetectors)
108+
if err != nil {
109+
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")
110+
}
111+
}
112+
scope := tfutils.RepoToScope(repoUrl)
113+
102114
fileWord := "file"
103115
if len(args) > 1 {
104116
fileWord = "files"
@@ -108,10 +120,9 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
108120

109121
plannedChanges := make([]*sdp.MappedItemDiff, 0)
110122

111-
lf := log.Fields{}
112123
for _, f := range args {
113124
lf["file"] = f
114-
result, err := tfutils.MappedItemDiffsFromPlanFile(ctx, f, lf)
125+
result, err := tfutils.MappedItemDiffsFromPlanFile(ctx, f, scope, lf)
115126
if err != nil {
116127
return loggedError{
117128
err: err,
@@ -136,14 +147,7 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
136147
title := changeTitle(viper.GetString("title"))
137148
tfPlanOutput := tryLoadText(ctx, viper.GetString("terraform-plan-output"))
138149
codeChangesOutput := tryLoadText(ctx, viper.GetString("code-changes-diff"))
139-
// Detect the repository URL if it wasn't provided
140-
repoUrl := viper.GetString("repo")
141-
if repoUrl == "" {
142-
repoUrl, err = DetectRepoURL(AllDetectors)
143-
if err != nil {
144-
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")
145-
}
146-
}
150+
147151
enrichedTags, err := parseTagsArgument()
148152
if err != nil {
149153
return loggedError{

‎cmd/repo.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ var AllDetectors = []RepoDetector{
1616
&RepoDetectorCircleCI{},
1717
&RepoDetectorAzureDevOps{},
1818
&RepoDetectorSpacelift{},
19+
&RepoDetectorScalr{},
1920
&RepoDetectorGitConfig{},
2021
}
2122

@@ -250,3 +251,29 @@ func (d *RepoDetectorGitConfig) DetectRepoURL(envVars map[string]string) (string
250251

251252
return "", fmt.Errorf("could not find remote URL in %v", gitConfigPath)
252253
}
254+
255+
type RepoDetectorScalr struct{}
256+
257+
func (d *RepoDetectorScalr) RequiredEnvVars() []string {
258+
return []string{"SCALR_WORKSPACE_NAME", "SCALR_ENVIRONMENT_NAME"}
259+
}
260+
261+
func (d *RepoDetectorScalr) DetectRepoURL(envVars map[string]string) (string, error) {
262+
workspaceName, ok := envVars["SCALR_WORKSPACE_NAME"]
263+
if !ok {
264+
return "", errors.New("SCALR_WORKSPACE_NAME not set")
265+
}
266+
267+
environmentName, ok := envVars["SCALR_ENVIRONMENT_NAME"]
268+
if !ok {
269+
return "", errors.New("SCALR_ENVIRONMENT_NAME not set")
270+
}
271+
272+
// A full Scalr URL can be constructed using
273+
// "https://$SCALR_HOSTNAME/v2/e/$SCALR_ENVIRONMENT_ID/workspaces/$SCALR_WORKSPACE_ID".
274+
// The problem with this is that the environment and workspace IDs are
275+
// computer generated and people aren't likely to understand what they mean.
276+
// Therefore we are going to go with custom URL scheme to make sure that the
277+
// URL is readable
278+
return fmt.Sprintf("scalr://%s/%s", environmentName, workspaceName), nil
279+
}

‎cmd/repo_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,3 +489,137 @@ func TestRepoDetectorGitConfig(t *testing.T) {
489489
}
490490
})
491491
}
492+
493+
func TestRepoDetectorScalr(t *testing.T) {
494+
t.Parallel()
495+
496+
t.Run("with valid SCALR_WORKSPACE_NAME and SCALR_ENVIRONMENT_NAME", func(t *testing.T) {
497+
t.Parallel()
498+
499+
envVars := map[string]string{
500+
"SCALR_WORKSPACE_NAME": "my-workspace",
501+
"SCALR_ENVIRONMENT_NAME": "production",
502+
}
503+
504+
detector := &RepoDetectorScalr{}
505+
506+
repoURL, err := detector.DetectRepoURL(envVars)
507+
if err != nil {
508+
t.Fatalf("unexpected error: %v", err)
509+
}
510+
511+
expectedRepoURL := "scalr://production/my-workspace"
512+
if repoURL != expectedRepoURL {
513+
t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
514+
}
515+
})
516+
517+
t.Run("with missing SCALR_WORKSPACE_NAME", func(t *testing.T) {
518+
t.Parallel()
519+
520+
envVars := map[string]string{
521+
"SCALR_ENVIRONMENT_NAME": "production",
522+
}
523+
524+
detector := &RepoDetectorScalr{}
525+
526+
repoURL, err := detector.DetectRepoURL(envVars)
527+
if err == nil {
528+
t.Fatal("expected error")
529+
}
530+
if repoURL != "" {
531+
t.Fatalf("expected empty repoURL, got %q", repoURL)
532+
}
533+
534+
expectedError := "SCALR_WORKSPACE_NAME not set"
535+
if err.Error() != expectedError {
536+
t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
537+
}
538+
})
539+
540+
t.Run("with missing SCALR_ENVIRONMENT_NAME", func(t *testing.T) {
541+
t.Parallel()
542+
543+
envVars := map[string]string{
544+
"SCALR_WORKSPACE_NAME": "my-workspace",
545+
}
546+
547+
detector := &RepoDetectorScalr{}
548+
549+
repoURL, err := detector.DetectRepoURL(envVars)
550+
if err == nil {
551+
t.Fatal("expected error")
552+
}
553+
if repoURL != "" {
554+
t.Fatalf("expected empty repoURL, got %q", repoURL)
555+
}
556+
557+
expectedError := "SCALR_ENVIRONMENT_NAME not set"
558+
if err.Error() != expectedError {
559+
t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
560+
}
561+
})
562+
563+
t.Run("with both variables missing", func(t *testing.T) {
564+
t.Parallel()
565+
566+
envVars := map[string]string{}
567+
568+
detector := &RepoDetectorScalr{}
569+
570+
repoURL, err := detector.DetectRepoURL(envVars)
571+
if err == nil {
572+
t.Fatal("expected error")
573+
}
574+
if repoURL != "" {
575+
t.Fatalf("expected empty repoURL, got %q", repoURL)
576+
}
577+
578+
expectedError := "SCALR_WORKSPACE_NAME not set"
579+
if err.Error() != expectedError {
580+
t.Fatalf("expected error to be %q, got %q", expectedError, err.Error())
581+
}
582+
})
583+
584+
t.Run("with empty values", func(t *testing.T) {
585+
t.Parallel()
586+
587+
envVars := map[string]string{
588+
"SCALR_WORKSPACE_NAME": "",
589+
"SCALR_ENVIRONMENT_NAME": "",
590+
}
591+
592+
detector := &RepoDetectorScalr{}
593+
594+
repoURL, err := detector.DetectRepoURL(envVars)
595+
if err != nil {
596+
t.Fatalf("unexpected error: %v", err)
597+
}
598+
599+
expectedRepoURL := "scalr:///"
600+
if repoURL != expectedRepoURL {
601+
t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
602+
}
603+
})
604+
605+
t.Run("with special characters", func(t *testing.T) {
606+
t.Parallel()
607+
608+
envVars := map[string]string{
609+
"SCALR_WORKSPACE_NAME": "my-workspace-with-dashes_and_underscores",
610+
"SCALR_ENVIRONMENT_NAME": "prod-env_123",
611+
}
612+
613+
detector := &RepoDetectorScalr{}
614+
615+
repoURL, err := detector.DetectRepoURL(envVars)
616+
if err != nil {
617+
t.Fatalf("unexpected error: %v", err)
618+
}
619+
620+
expectedRepoURL := "scalr://prod-env_123/my-workspace-with-dashes_and_underscores"
621+
if repoURL != expectedRepoURL {
622+
t.Fatalf("expected repoURL to be %q, got %q", expectedRepoURL, repoURL)
623+
}
624+
})
625+
}

‎cmd/terraform_plan.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
127127

128128
removingSecretsSpinner.Success()
129129

130+
// Detect the repository URL if it wasn't provided
131+
repoUrl := viper.GetString("repo")
132+
if repoUrl == "" {
133+
repoUrl, _ = DetectRepoURL(AllDetectors)
134+
}
135+
130136
///////////////////////////////////////////////////////////////////
131137
// Extract changes from the plan and created mapped item diffs
132138
///////////////////////////////////////////////////////////////////
@@ -135,8 +141,10 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
135141
resourceExtractionResults := multi.NewWriter()
136142
time.Sleep(200 * time.Millisecond) // give the UI a little time to update
137143

144+
scope := tfutils.RepoToScope(repoUrl)
145+
138146
// Map the terraform changes to Overmind queries
139-
mappingResponse, err := tfutils.MappedItemDiffsFromPlan(ctx, planJson, planFile, log.Fields{})
147+
mappingResponse, err := tfutils.MappedItemDiffsFromPlan(ctx, planJson, planFile, scope, log.Fields{})
140148
if err != nil {
141149
resourceExtractionSpinner.Fail(fmt.Sprintf("Removing secrets: %v", err))
142150
return nil
@@ -230,11 +238,6 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
230238

231239
codeChangesOutput := tryLoadText(ctx, viper.GetString("code-changes-diff"))
232240

233-
// Detect the repository URL if it wasn't provided
234-
repoUrl := viper.GetString("repo")
235-
if repoUrl == "" {
236-
repoUrl, _ = DetectRepoURL(AllDetectors)
237-
}
238241
enrichedTags, err := parseTagsArgument()
239242
if err != nil {
240243
uploadChangesSpinner.Fail(fmt.Sprintf("Uploading planned changes: failed to parse tags: %v", err))

‎tfutils/plan_mapper.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,15 @@ func (r *PlanMappingResult) numStatus(status MapStatus) int {
106106
return count
107107
}
108108

109-
func MappedItemDiffsFromPlanFile(ctx context.Context, fileName string, lf log.Fields) (*PlanMappingResult, error) {
109+
func MappedItemDiffsFromPlanFile(ctx context.Context, fileName string, scope string, lf log.Fields) (*PlanMappingResult, error) {
110110
// read results from `terraform show -json ${tfplan file}`
111111
planJSON, err := os.ReadFile(fileName)
112112
if err != nil {
113113
log.WithContext(ctx).WithError(err).WithFields(lf).Error("Failed to read terraform plan")
114114
return nil, err
115115
}
116116

117-
return MappedItemDiffsFromPlan(ctx, planJSON, fileName, lf)
117+
return MappedItemDiffsFromPlan(ctx, planJSON, fileName, scope, lf)
118118
}
119119

120120
type TfMapData struct {
@@ -135,8 +135,9 @@ type TfMapData struct {
135135
// and current resource values. The function categorizes the mapped item
136136
// differences into supported and unsupported changes. Finally, it logs the
137137
// number of supported and unsupported changes and returns the mapped item
138-
// differences.
139-
func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName string, lf log.Fields) (*PlanMappingResult, error) {
138+
// differences. The `scope` determines the scope of the resources that will be
139+
// generated, not the queries. These will always have a scope of `*`
140+
func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName string, scope string, lf log.Fields) (*PlanMappingResult, error) {
140141
// Create a span for this since we're going to be attaching events to it when things fail to map
141142
span := trace.SpanFromContext(ctx)
142143
defer span.End()
@@ -195,7 +196,7 @@ func MappedItemDiffsFromPlan(ctx context.Context, planJson []byte, fileName stri
195196
continue
196197
}
197198

198-
itemDiff, err := itemDiffFromResourceChange(resourceChange)
199+
itemDiff, err := itemDiffFromResourceChange(resourceChange, scope)
199200
if err != nil {
200201
return nil, fmt.Errorf("failed to create item diff for resource change: %w", err)
201202
}
@@ -440,9 +441,7 @@ func countSensitiveAttributes(attributes, sensitive any) int {
440441
}
441442

442443
// Converts a ResourceChange form a terraform plan to an ItemDiff in SDP format.
443-
// These items will use the scope `terraform_plan` since we haven't mapped them
444-
// to an actual item in the infrastructure yet
445-
func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, error) {
444+
func itemDiffFromResourceChange(resourceChange ResourceChange, scope string) (*sdp.ItemDiff, error) {
446445
status := sdp.ItemDiffStatus_ITEM_DIFF_STATUS_UNSPECIFIED
447446

448447
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
488487
Type: resourceChange.Type,
489488
UniqueAttribute: "terraform_name",
490489
Attributes: beforeAttributes,
491-
Scope: "terraform_plan",
490+
Scope: scope,
492491
}
493492

494493
err = result.GetBefore().GetAttributes().Set("terraform_name", trimmedAddress)
@@ -509,7 +508,7 @@ func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, e
509508
Type: resourceChange.Type,
510509
UniqueAttribute: "terraform_name",
511510
Attributes: afterAttributes,
512-
Scope: "terraform_plan",
511+
Scope: scope,
513512
}
514513

515514
err = result.GetAfter().GetAttributes().Set("terraform_name", trimmedAddress)

‎tfutils/plan_mapper_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
)
1616

1717
func TestWithStateFile(t *testing.T) {
18-
_, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/state.json", log.Fields{})
18+
_, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/state.json", "scope", log.Fields{})
1919

2020
if err == nil {
2121
t.Error("Expected error when running with state file, got none")
@@ -48,7 +48,7 @@ func TestExtractProviderNameFromConfigKey(t *testing.T) {
4848
}
4949

5050
func TestMappedItemDiffsFromPlan(t *testing.T) {
51-
results, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/plan.json", log.Fields{})
51+
results, err := MappedItemDiffsFromPlanFile(context.Background(), "testdata/plan.json", "scope", log.Fields{})
5252
if err != nil {
5353
t.Error(err)
5454
}
@@ -120,8 +120,8 @@ func TestMappedItemDiffsFromPlan(t *testing.T) {
120120
if nats_box_deployment.GetMappingQuery().GetScope() != "*" {
121121
t.Errorf("Expected nats_box_deployment query scope to be '*', got '%v'", nats_box_deployment.GetMappingQuery().GetScope())
122122
}
123-
if nats_box_deployment.GetItem().GetBefore().GetScope() != "terraform_plan" {
124-
t.Errorf("Expected nats_box_deployment before item scope to be 'terraform_plan', got '%v'", nats_box_deployment.GetItem().GetBefore().GetScope())
123+
if nats_box_deployment.GetItem().GetBefore().GetScope() != "scope" {
124+
t.Errorf("Expected nats_box_deployment before item scope to be 'scope', got '%v'", nats_box_deployment.GetItem().GetBefore().GetScope())
125125
}
126126
if nats_box_deployment.GetMappingQuery().GetType() != "Deployment" {
127127
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) {
150150
if api_server_deployment.GetMappingQuery().GetScope() != "*" {
151151
t.Errorf("Expected api_server_deployment query scope to be '*', got '%v'", api_server_deployment.GetMappingQuery().GetScope())
152152
}
153-
if api_server_deployment.GetItem().GetBefore().GetScope() != "terraform_plan" {
154-
t.Errorf("Expected api_server_deployment before item scope to be 'terraform_plan', got '%v'", api_server_deployment.GetItem().GetBefore().GetScope())
153+
if api_server_deployment.GetItem().GetBefore().GetScope() != "scope" {
154+
t.Errorf("Expected api_server_deployment before item scope to be 'scope', got '%v'", api_server_deployment.GetItem().GetBefore().GetScope())
155155
}
156156
if api_server_deployment.GetMappingQuery().GetType() != "Deployment" {
157157
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) {
180180
if aws_iam_policy.GetMappingQuery().GetScope() != "*" {
181181
t.Errorf("Expected aws_iam_policy query scope to be '*', got '%v'", aws_iam_policy.GetMappingQuery().GetScope())
182182
}
183-
if aws_iam_policy.GetItem().GetBefore().GetScope() != "terraform_plan" {
184-
t.Errorf("Expected aws_iam_policy before item scope to be 'terraform_plan', got '%v'", aws_iam_policy.GetItem().GetBefore().GetScope())
183+
if aws_iam_policy.GetItem().GetBefore().GetScope() != "scope" {
184+
t.Errorf("Expected aws_iam_policy before item scope to be 'scope', got '%v'", aws_iam_policy.GetItem().GetBefore().GetScope())
185185
}
186186
if aws_iam_policy.GetMappingQuery().GetType() != "iam-policy" {
187187
t.Errorf("Expected aws_iam_policy query type to be 'iam-policy', got '%v'", aws_iam_policy.GetMappingQuery().GetType())

0 commit comments

Comments
 (0)