diff --git a/extract/secret.go b/extract/secret.go deleted file mode 100644 index 7d5e626..0000000 --- a/extract/secret.go +++ /dev/null @@ -1,92 +0,0 @@ -package extract - -import ( - "fmt" - - "github.com/aquasecurity/trivy/pkg/iac/terraform" - "github.com/hashicorp/hcl/v2" - - "github.com/coder/preview/types" -) - -// SecretFromBlock decodes a `data "coder_secret" {}` Terraform block into a -// SecretRequirement. Exactly one of `env` or `file` must be set, and -// `help_message` is required. Returns (nil, diags) on validation failure. -func SecretFromBlock(block *terraform.Block) (req *types.SecretRequirement, diags hcl.Diagnostics) { - defer func() { - // Extra safety mechanism to ensure that if a panic occurs, we do not break - // everything else. - if r := recover(); r != nil { - req = nil - diags = hcl.Diagnostics{ - { - Severity: hcl.DiagError, - Summary: "Panic occurred in extracting secret requirement. This should not happen, please report this to Coder.", - Detail: fmt.Sprintf("panic in secret extract: %+v", r), - }, - } - } - }() - - // help_message is required AND must be a string; requiredString - // handles both checks and emits a proper type diagnostic. - help, helpDiag := requiredString(block, "help_message") - if helpDiag != nil { - diags = diags.Append(helpDiag) - } - - // Check presence separately from value so we can distinguish "attribute - // absent" from "attribute present but wrong type"; the latter must produce - // a type diagnostic instead of being silently treated as unset. - envAttr := block.GetAttribute("env") - fileAttr := block.GetAttribute("file") - envSet := envAttr != nil && !envAttr.IsNil() - fileSet := fileAttr != nil && !fileAttr.IsNil() - - var env, file string - if envSet { - v, d := requiredString(block, "env") - if d != nil { - diags = diags.Append(d) - } - env = v - } - if fileSet { - v, d := requiredString(block, "file") - if d != nil { - diags = diags.Append(d) - } - file = v - } - - // Mutual exclusivity is based on presence, not parsed value, so a - // wrong-type attribute still counts as "set" here. - switch { - case !envSet && !fileSet: - r := block.HCLBlock().DefRange - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: `Invalid "coder_secret" block`, - Detail: `Exactly one of "env" or "file" must be set, neither were set`, - Subject: &r, - }) - case envSet && fileSet: - r := block.HCLBlock().DefRange - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: `Invalid "coder_secret" block`, - Detail: `Exactly one of "env" or "file" must be set, both were set`, - Subject: &r, - }) - } - - if diags.HasErrors() { - return nil, diags - } - - return &types.SecretRequirement{ - Env: env, - File: file, - HelpMessage: help, - }, diags -} diff --git a/extract/secret_test.go b/extract/secret_test.go deleted file mode 100644 index 213e097..0000000 --- a/extract/secret_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package extract_test - -import ( - "strings" - "testing" - - "github.com/hashicorp/hcl/v2" - "github.com/stretchr/testify/require" - - "github.com/coder/preview/extract" -) - -// Test_SecretFromBlock_PanicRecover verifies that a panic inside -// SecretFromBlock is converted into an error diagnostic rather than crashing -// the whole extraction pass. A nil block triggers a nil pointer dereference -// inside requiredString, which the deferred recover should catch. -func Test_SecretFromBlock_PanicRecover(t *testing.T) { - t.Parallel() - - req, diags := extract.SecretFromBlock(nil) - require.Nil(t, req) - require.True(t, diags.HasErrors(), "expected diagnostics; got %v", diags) - - var found bool - for _, d := range diags { - if d.Severity == hcl.DiagError && strings.Contains(d.Summary, "Panic occurred in extracting secret requirement") { - found = true - break - } - } - require.True(t, found, "expected panic diagnostic; got: %v", diags) -} diff --git a/preview.go b/preview.go index 3490dc4..32ac43f 100644 --- a/preview.go +++ b/preview.go @@ -40,11 +40,10 @@ type Output struct { // JSON marshalling is handled in the custom methods. ModuleOutput cty.Value `json:"-"` - Parameters []types.Parameter `json:"parameters"` - WorkspaceTags types.TagBlocks `json:"workspace_tags"` - Presets []types.Preset `json:"presets"` - Variables []types.Variable `json:"variables"` - SecretRequirements []types.SecretRequirement `json:"secret_requirements"` + Parameters []types.Parameter `json:"parameters"` + WorkspaceTags types.TagBlocks `json:"workspace_tags"` + Presets []types.Preset `json:"presets"` + Variables []types.Variable `json:"variables"` // Files is included for printing diagnostics. // They can be marshalled, but not unmarshalled. This is a limitation // of the HCL library. @@ -280,20 +279,18 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn preValidPresets := presets(modules, rp) tags, tagDiags := workspaceTags(modules, p.Files()) vars := variables(modules) - secretReqs, secretDiags := secrets(modules) // Add warnings diags = diags.Extend(warnings(modules)) return &Output{ - ModuleOutput: outputs, - Parameters: rp, - WorkspaceTags: tags, - Presets: preValidPresets, - Files: p.Files(), - Variables: vars, - SecretRequirements: secretReqs, - }, diags.Extend(overrideDiags).Extend(rpDiags).Extend(tagDiags).Extend(secretDiags) + ModuleOutput: outputs, + Parameters: rp, + WorkspaceTags: tags, + Presets: preValidPresets, + Files: p.Files(), + Variables: vars, + }, diags.Extend(overrideDiags).Extend(rpDiags).Extend(tagDiags) } func (i Input) RichParameterValue(key string) (string, bool) { diff --git a/preview_test.go b/preview_test.go index 64dc2c1..49a7fb6 100644 --- a/preview_test.go +++ b/preview_test.go @@ -9,7 +9,6 @@ import ( "slices" "strings" "testing" - "testing/fstest" "github.com/hashicorp/hcl/v2" "github.com/stretchr/testify/assert" @@ -42,14 +41,13 @@ func Test_Extract(t *testing.T) { failPreview bool input preview.Input - expTags map[string]string - unknownTags []string - params map[string]assertParam - variables map[string]assertVariable - presetsFuncs func(t *testing.T, presets []types.Preset) - presets map[string]assertPreset - warnings []*regexp.Regexp - secretRequirements []types.SecretRequirement + expTags map[string]string + unknownTags []string + params map[string]assertParam + variables map[string]assertVariable + presetsFuncs func(t *testing.T, presets []types.Preset) + presets map[string]assertPreset + warnings []*regexp.Regexp }{ { name: "bad param values", @@ -659,38 +657,6 @@ func Test_Extract(t *testing.T) { prebuildCount(1), }, }, - { - name: "secrets basic", - dir: "secretsbasic", - secretRequirements: []types.SecretRequirement{ - {Env: "GITHUB_TOKEN", HelpMessage: "Add a GitHub PAT"}, - {File: "~/.aws/credentials", HelpMessage: "Add AWS creds"}, - }, - }, - { - name: "secrets conditional off", - dir: "secretsconditional", - input: preview.Input{ - ParameterValues: map[string]string{"use_github": "false"}, - }, - params: map[string]assertParam{ - "use_github": ap().value("false"), - }, - secretRequirements: nil, - }, - { - name: "secrets conditional on", - dir: "secretsconditional", - input: preview.Input{ - ParameterValues: map[string]string{"use_github": "true"}, - }, - params: map[string]assertParam{ - "use_github": ap().value("true"), - }, - secretRequirements: []types.SecretRequirement{ - {Env: "GITHUB_TOKEN", HelpMessage: "Add a GitHub PAT"}, - }, - }, { name: "override", dir: "override", @@ -790,10 +756,6 @@ func Test_Extract(t *testing.T) { require.True(t, ok, "unknown variable %s", variable.Name) check(t, variable) } - - // Assert secret requirements - require.ElementsMatch(t, tc.secretRequirements, output.SecretRequirements, - "secret requirements do not match expected") }) } } @@ -1143,172 +1105,3 @@ DiagLoop: assert.Equal(t, []string{}, checks, "missing expected diagnostic errors") } - -func Test_SecretRequirementErrors(t *testing.T) { - t.Parallel() - tests := []struct { - name string - tf string - wantDiag string // substring match on summary+" "+detail - }{ - { - name: "missing help_message", - tf: ` -data "coder_secret" "x" { - env = "X" -} -`, - wantDiag: `help_message`, - }, - { - name: "help_message null", - tf: ` -data "coder_secret" "x" { - env = "X" - help_message = null -} -`, - wantDiag: `help_message`, - }, - { - name: "help_message wrong type (number)", - tf: ` -data "coder_secret" "x" { - env = "X" - help_message = 42 -} -`, - wantDiag: `Expected a string`, - }, - { - name: "neither env nor file", - tf: ` -data "coder_secret" "x" { - help_message = "need one" -} -`, - wantDiag: `Exactly one of "env" or "file" must be set`, - }, - { - name: "both env and file", - tf: ` -data "coder_secret" "x" { - env = "X" - file = "~/y" - help_message = "ok" -} -`, - wantDiag: `Exactly one of "env" or "file" must be set`, - }, - { - name: "env wrong type (number)", - tf: ` -data "coder_secret" "x" { - env = 42 - help_message = "ok" -} -`, - wantDiag: `Expected a string`, - }, - { - name: "file wrong type (bool)", - tf: ` -data "coder_secret" "x" { - file = true - help_message = "ok" -} -`, - wantDiag: `Expected a string`, - }, - { - name: "env null", - tf: ` -data "coder_secret" "x" { - env = null - help_message = "ok" -} -`, - wantDiag: `Expected a string`, - }, - { - name: "file null", - tf: ` -data "coder_secret" "x" { - file = null - help_message = "ok" -} -`, - wantDiag: `Expected a string`, - }, - { - name: "duplicate block labels", - tf: ` -data "coder_secret" "x" { - env = "A" - help_message = "first" -} -data "coder_secret" "x" { - env = "B" - help_message = "second" -} -`, - wantDiag: `duplicate coder_secret blocks with name "x"`, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - fsys := fstest.MapFS{"main.tf": &fstest.MapFile{Data: []byte(tc.tf)}} - _, diags := preview.Preview(context.Background(), preview.Input{}, fsys) - require.True(t, diags.HasErrors(), "expected errors; got %v", diags) - var found bool - for _, d := range diags { - if strings.Contains(d.Summary+" "+d.Detail, tc.wantDiag) { - found = true - break - } - } - require.True(t, found, - "no diag matching %q; got: %v", tc.wantDiag, diags) - }) - } -} - -// Test_SecretRequirement_DuplicateDetectionRequiresExtractionSuccess pins the -// current behavior: duplicate-label detection only fires for blocks that -// successfully extract. When one of two same-labeled blocks has an extraction -// error (e.g. wrong attribute type), the dedup bookkeeping skips it, so the -// "duplicate coder_secret blocks" diagnostic does not appear — only the -// per-block extraction error does. This matches parameter.go's precedent. -// If this behavior is ever changed to "track duplicates regardless of -// extraction success," update this test. -func Test_SecretRequirement_DuplicateDetectionRequiresExtractionSuccess(t *testing.T) { - t.Parallel() - tf := ` -data "coder_secret" "x" { - env = 42 - help_message = "first" -} -data "coder_secret" "x" { - env = "B" - help_message = "second" -} -` - fsys := fstest.MapFS{"main.tf": &fstest.MapFile{Data: []byte(tf)}} - _, diags := preview.Preview(context.Background(), preview.Input{}, fsys) - require.True(t, diags.HasErrors(), "expected errors; got %v", diags) - - var hasTypeErr, hasDupeErr bool - for _, d := range diags { - msg := d.Summary + " " + d.Detail - if strings.Contains(msg, "Expected a string") { - hasTypeErr = true - } - if strings.Contains(msg, "duplicate coder_secret blocks") { - hasDupeErr = true - } - } - require.True(t, hasTypeErr, "expected type error for env=42; got: %v", diags) - require.False(t, hasDupeErr, - "duplicate diagnostic unexpectedly fired; dedup should only track successful extractions. diags: %v", diags) -} diff --git a/secret.go b/secret.go deleted file mode 100644 index a918102..0000000 --- a/secret.go +++ /dev/null @@ -1,56 +0,0 @@ -package preview - -import ( - "fmt" - "strings" - - "github.com/aquasecurity/trivy/pkg/iac/terraform" - "github.com/hashicorp/hcl/v2" - - "github.com/coder/preview/extract" - "github.com/coder/preview/types" -) - -func secrets(modules terraform.Modules) ([]types.SecretRequirement, hcl.Diagnostics) { - diags := make(hcl.Diagnostics, 0) - reqs := make([]types.SecretRequirement, 0) - // Track blocks by label (e.g. "x" in `data "coder_secret" "x"`) so we can - // emit a single duplicate diagnostic per colliding label. Mirrors the - // parameter dedup pattern in parameter.go. - exists := make(map[string][]*terraform.Block) - - for _, mod := range modules { - blocks := mod.GetDatasByType(types.BlockTypeSecret) - for _, block := range blocks { - req, rDiags := extract.SecretFromBlock(block) - if len(rDiags) > 0 { - diags = diags.Extend(rDiags) - } - if req != nil { - reqs = append(reqs, *req) - name := block.NameLabel() - exists[name] = append(exists[name], block) - } - } - } - - for name, blocks := range exists { - if len(blocks) <= 1 { - continue - } - var detail strings.Builder - for _, b := range blocks { - _, _ = detail.WriteString(fmt.Sprintf("block %q at %s\n", - b.Type()+"."+strings.Join(b.Labels(), "."), - b.HCLBlock().TypeRange)) - } - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Found %d duplicate coder_secret blocks with name %q, this is not allowed", len(blocks), name), - Detail: detail.String(), - }) - } - - types.SortSecretRequirements(reqs) - return reqs, diags -} diff --git a/testdata/secretsbasic/main.tf b/testdata/secretsbasic/main.tf deleted file mode 100644 index 4a35e09..0000000 --- a/testdata/secretsbasic/main.tf +++ /dev/null @@ -1,18 +0,0 @@ -terraform { - required_providers { - coder = { - source = "coder/coder" - version = "2.16.0" - } - } -} - -data "coder_secret" "gh" { - env = "GITHUB_TOKEN" - help_message = "Add a GitHub PAT" -} - -data "coder_secret" "aws" { - file = "~/.aws/credentials" - help_message = "Add AWS creds" -} diff --git a/testdata/secretsconditional/main.tf b/testdata/secretsconditional/main.tf deleted file mode 100644 index 671f706..0000000 --- a/testdata/secretsconditional/main.tf +++ /dev/null @@ -1,21 +0,0 @@ -terraform { - required_providers { - coder = { - source = "coder/coder" - version = "2.16.0" - } - } -} - -data "coder_parameter" "use_github" { - name = "use_github" - type = "bool" - default = "false" - mutable = true -} - -data "coder_secret" "gh" { - count = data.coder_parameter.use_github.value == "true" ? 1 : 0 - env = "GITHUB_TOKEN" - help_message = "Add a GitHub PAT" -} diff --git a/types/secret.go b/types/secret.go deleted file mode 100644 index feb5c7b..0000000 --- a/types/secret.go +++ /dev/null @@ -1,29 +0,0 @@ -package types - -import ( - "slices" - "strings" -) - -// @typescript-ignore BlockTypeSecret -const BlockTypeSecret = "coder_secret" - -// SecretRequirement describes a `data "coder_secret"` block declared in a -// template. Exactly one of Env or File will be non-empty; validation of that -// invariant happens during extraction. -type SecretRequirement struct { - Env string `json:"env,omitempty"` - File string `json:"file,omitempty"` - HelpMessage string `json:"help_message,omitempty"` -} - -// SortSecretRequirements orders requirements first by Env then by File so -// diagnostic output is stable across runs. -func SortSecretRequirements(reqs []SecretRequirement) { - slices.SortFunc(reqs, func(a, b SecretRequirement) int { - if c := strings.Compare(a.Env, b.Env); c != 0 { - return c - } - return strings.Compare(a.File, b.File) - }) -} diff --git a/types/secret_test.go b/types/secret_test.go deleted file mode 100644 index a3e0846..0000000 --- a/types/secret_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package types_test - -import ( - "encoding/json" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/coder/preview/types" -) - -// Test_SecretRequirement_JSON_Omitempty verifies that empty Env, File, and -// HelpMessage fields are omitted from the marshaled JSON. Since extraction -// enforces exactly one of Env/File is non-empty, omitempty makes the on-the- -// wire shape explicit about which field applies. -func Test_SecretRequirement_JSON_Omitempty(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - req types.SecretRequirement - want string - }{ - { - name: "env only", - req: types.SecretRequirement{Env: "FOO", HelpMessage: "set FOO"}, - want: `{"env":"FOO","help_message":"set FOO"}`, - }, - { - name: "file only", - req: types.SecretRequirement{File: "~/bar", HelpMessage: "set bar"}, - want: `{"file":"~/bar","help_message":"set bar"}`, - }, - { - name: "empty help_message is omitted", - req: types.SecretRequirement{Env: "FOO"}, - want: `{"env":"FOO"}`, - }, - { - name: "all empty produces empty object", - req: types.SecretRequirement{}, - want: `{}`, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got, err := json.Marshal(tc.req) - require.NoError(t, err) - require.JSONEq(t, tc.want, string(got)) - }) - } -}