Skip to content

Always regen _massdriver_variables.tf on bundle build #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrisghill merged 3 commits into from
Mar 12, 2026
Merged

Always regen _massdriver_variables.tf on bundle build #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e47158
Always regen _massdriver_variables.tf on bundle build
chrisghill Mar 12, 2026
a3c1485
PR findings
chrisghill Mar 12, 2026
27f770d
PR finding
chrisghill Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions pkg/bundle/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ var expectedSchemaContents = map[string][]byte{
}

var expectedTFContent = map[string][]byte{
"_massdriver_variables.tf": []byte(`// Auto-generated variable declarations from massdriver.yaml
"_massdriver_variables.tf": []byte(`// This file is auto-generated by massdriver from your massdriver.yaml file.
// Any changes made directly to this file will be overwritten on the next build.
// To opt a variable out of regeneration, move it to another file (e.g. variables.tf).
variable "draft_node_foo" {
type = object({
foo = optional(object({
Expand All @@ -176,13 +178,7 @@ variable "foo" {
}
variable "md_metadata" {
type = object({
default_tags = object({
managed-by = string
md-manifest = string
md-package = string
md-project = string
md-target = string
})
default_tags = map(string)
deployment = object({
id = string
})
Expand Down
11 changes: 3 additions & 8 deletions pkg/bundle/combine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,10 @@ import (
var mdMetadataMap = map[string]any{
"properties": map[string]any{
"default_tags": map[string]any{
"properties": map[string]any{
"managed-by": map[string]any{"type": "string"},
"md-manifest": map[string]any{"type": "string"},
"md-package": map[string]any{"type": "string"},
"md-project": map[string]any{"type": "string"},
"md-target": map[string]any{"type": "string"},
"type": "object",
"additionalProperties": map[string]any{
"type": "string",
},
"required": []any{"managed-by", "md-manifest", "md-package", "md-project", "md-target"},
"type": "object",
},
"deployment": map[string]any{
"properties": map[string]any{
Expand Down
27 changes: 3 additions & 24 deletions pkg/bundle/schemas/metadata-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,9 @@
"properties": {
"default_tags": {
"type": "object",
"properties": {
"managed-by": {
"type": "string"
},
"md-manifest": {
"type": "string"
},
"md-package": {
"type": "string"
},
"md-project": {
"type": "string"
},
"md-target": {
"type": "string"
}
},
"required": [
"managed-by",
"md-manifest",
"md-package",
"md-project",
"md-target"
]
"additionalProperties": {
"type": "string"
}
},
"deployment": {
"type": "object",
Expand Down
39 changes: 27 additions & 12 deletions pkg/provisioners/opentofu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,30 @@ import (

type OpentofuProvisioner struct{}

func (p *OpentofuProvisioner) ExportMassdriverInputs(stepPath string, variables map[string]any) error {
// read existing OpenTofu variables for this step
func (p *OpentofuProvisioner) ExportMassdriverInputs(stepPath string, variables map[string]any) (retErr error) {
massdriverVarsFile := path.Join(stepPath, "_massdriver_variables.tf")
massdriverVarsBackup := massdriverVarsFile + ".bak"

// If _massdriver_variables.tf already exists, rename it so airlock won't read it
// during the scan. This allows variables declared there to be regenerated.
// The defer below restores the backup on any error, or removes it on success.
if _, statErr := os.Stat(massdriverVarsFile); statErr == nil {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

massdriverVarsBackup is always the fixed path "_massdriver_variables.tf.bak". If that backup already exists (e.g., a prior run crashed after the rename), os.Rename may fail on Windows (and can overwrite on Unix). Consider removing/archiving any existing backup first or using a unique backup name (PID/timestamp) to avoid collisions and cross-platform behavior differences.

Suggested change
if _, statErr := os.Stat(massdriverVarsFile); statErr == nil {
if _, statErr := os.Stat(massdriverVarsFile); statErr == nil {
// Ensure any existing backup does not cause cross-platform rename issues.
if _, backupStatErr := os.Stat(massdriverVarsBackup); backupStatErr == nil {
if removeErr := os.Remove(massdriverVarsBackup); removeErr != nil {
return removeErr
}
} else if !errors.Is(backupStatErr, os.ErrNotExist) {
return backupStatErr
}

Copilot uses AI. Check for mistakes.
if renameErr := os.Rename(massdriverVarsFile, massdriverVarsBackup); renameErr != nil {
return renameErr
}
defer func() {
if retErr != nil {
os.Remove(massdriverVarsFile) //nolint:errcheck
os.Rename(massdriverVarsBackup, massdriverVarsFile) //nolint:errcheck
} else {
os.Remove(massdriverVarsBackup) //nolint:errcheck
}
}()
} else if !errors.Is(statErr, os.ErrNotExist) {
return statErr
}

// read existing OpenTofu variables for this step (excludes _massdriver_variables.tf)
tofuVarsImport := opentofu.TofuToSchema(stepPath)
if tofuVarsImport.Schema == nil {
return errors.New("failed to read existing OpenTofu variable declarations: " + tofuVarsImport.PrettyDiags())
Expand All @@ -34,17 +56,10 @@ func (p *OpentofuProvisioner) ExportMassdriverInputs(stepPath string, variables
return transpileErr
}

comment := []byte("// Auto-generated variable declarations from massdriver.yaml\n")
content = append(comment, content...)
filename := "/_massdriver_variables.tf"
fh, openErr := os.OpenFile(path.Join(stepPath, filename), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if openErr != nil {
return openErr
}
defer fh.Close()
header := []byte("// This file is auto-generated by massdriver from your massdriver.yaml file.\n// Any changes made directly to this file will be overwritten on the next build.\n// To opt a variable out of regeneration, move it to another file (e.g. variables.tf).\n")
content = append(header, content...)

_, writeErr := fh.Write(content)
if writeErr != nil {
if writeErr := os.WriteFile(massdriverVarsFile, content, 0644); writeErr != nil {
return writeErr
}

Expand Down
80 changes: 69 additions & 11 deletions pkg/provisioners/opentofu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,16 @@ import (
"github.com/massdriver-cloud/mass/pkg/provisioners"
)

const autoGeneratedHeader = "// This file is auto-generated by massdriver from your massdriver.yaml file.\n// Any changes made directly to this file will be overwritten on the next build.\n// To opt a variable out of regeneration, move it to another file (e.g. variables.tf).\n"
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoGeneratedHeader is now duplicated across multiple tests (and the provisioner implementation embeds the same header string). This is easy to let drift and causes brittle string-matching assertions. Consider defining the header once in the provisioners package (const) and reusing it in both the implementation and tests.

Copilot uses AI. Check for mistakes.

func TestOpentofuExportMassdriverInputs(t *testing.T) {
type test struct {
name string
variables map[string]any
want string
errString string
name string
tfFile string // testdata fixture to use as variables.tf; defaults to tc.name
variables map[string]any
existingMassdriverVars string // pre-populate _massdriver_variables.tf with this content
want string
errString string
}
tests := []test{
{
Expand Down Expand Up @@ -49,12 +53,54 @@ func TestOpentofuExportMassdriverInputs(t *testing.T) {
},
},
},
want: `// Auto-generated variable declarations from massdriver.yaml
variable "bar" {
want: autoGeneratedHeader + `variable "bar" {
type = string
}
`,
},
Comment on lines 28 to +60
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test fixtures used here don't appear to match the scenarios the test names/expectations describe. For example, testdata/opentofu/same.tf currently declares only "foo", but this case's md schema includes both "foo" and "bar" and expects no _massdriver_variables.tf output. Conversely, missingopentofu expects bar to be generated, but its fixture currently declares both variables. This makes these cases fail or test the wrong behavior; consider using the new tfFile field to point each case at the intended fixture (or update the fixtures to match the names).

Copilot uses AI. Check for mistakes.
{
name: "regenerate",
tfFile: "missingopentofu",
variables: map[string]any{
"required": []any{"bar", "foo"},
"properties": map[string]any{
"bar": map[string]any{
"type": "string",
},
"foo": map[string]any{
"type": "string",
},
},
},
existingMassdriverVars: autoGeneratedHeader + `variable "bar" {
type = string
}
`,
want: autoGeneratedHeader + `variable "bar" {
type = string
}
`,
},
{
name: "regenerateclean",
tfFile: "same",
variables: map[string]any{
"required": []any{"bar", "foo"},
"properties": map[string]any{
"bar": map[string]any{
"type": "string",
},
"foo": map[string]any{
"type": "string",
},
},
},
existingMassdriverVars: autoGeneratedHeader + `variable "bar" {
type = string
}
`,
want: ``,
},
{
name: "missingmassdriver",
variables: map[string]any{
Expand All @@ -77,14 +123,26 @@ variable "bar" {
t.Run(tc.name, func(t *testing.T) {
testDir := t.TempDir()

content, err := os.ReadFile(path.Join("testdata", "opentofu", fmt.Sprintf("%s.tf", tc.name)))
tfFile := tc.tfFile
if tfFile == "" {
tfFile = tc.name
}

content, err := os.ReadFile(path.Join("testdata", "opentofu", fmt.Sprintf("%s.tf", tfFile)))
if err != nil {
t.Fatalf("%d, unexpected error", err)
t.Fatalf("unexpected error: %v", err)
}

err = os.WriteFile(path.Join(testDir, "variables.tf"), content, 0644)
if err != nil {
t.Fatalf("%d, unexpected error", err)
t.Fatalf("unexpected error: %v", err)
}

if tc.existingMassdriverVars != "" {
err = os.WriteFile(path.Join(testDir, "_massdriver_variables.tf"), []byte(tc.existingMassdriverVars), 0644)
if err != nil {
t.Fatalf("unexpected error writing existing massdriver vars: %v", err)
}
}

prov := provisioners.OpentofuProvisioner{}
Expand Down Expand Up @@ -150,12 +208,12 @@ func TestOpentofuReadProvisionerInputs(t *testing.T) {

content, err := os.ReadFile(path.Join("testdata", "opentofu", fmt.Sprintf("%s.tf", tc.name)))
if err != nil {
t.Fatalf("%d, unexpected error", err)
t.Fatalf("unexpected error: %v", err)
}

err = os.WriteFile(path.Join(testDir, "variables.tf"), content, 0644)
if err != nil {
t.Fatalf("%d, unexpected error", err)
t.Fatalf("unexpected error: %v", err)
}

prov := provisioners.OpentofuProvisioner{}
Expand Down
Loading