Skip to content

feat: load variables from tfvars files #11549

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.

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
merged 19 commits into from
Jan 12, 2024
Merged

feat: load variables from tfvars files #11549

merged 19 commits into from
Jan 12, 2024

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Jan 10, 2024

Fixes: #8501

This PR expands the template push and template create to review .tfvars and .tfvars.json files while uploading next template version. CLI will support a flat variable structure:

HCL

region = "us-east-1"
cores = 2
go_image = ["1.19","1.20","1.21"]

JSON

{
    "region": "us-east-1",
    "cores": 2,
    "go_image": ["1.19","1.20","1.21"]
}

TODO:

  • test: DiscoverVarsFiles
  • test: ParseUserVariableValues

@mtojek mtojek requested a review from johnstcn January 12, 2024 12:37
@mtojek mtojek marked this pull request as ready for review January 12, 2024 12:37
Comment on lines +112 to +115
varsFiles, err = DiscoverVarsFiles(uploadFlags.directory)
if err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

I could imagine a case where someone had tfvars files lying around unused before, worked around the issue, and left them there. Now we're going to auto-discover them. I'm not sure what this will break.
Should at the very least add an info message about the auto-discovered vars files?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I changed the code to print a message alerting about the presence of tfvars 👍

stringData[attribute.Name] = ctyValue.AsString()
} else if ctyType.Equals(cty.Number) {
stringData[attribute.Name] = ctyValue.AsBigFloat().String()
} else if ctyType.IsTupleType() {
Copy link
Member

Choose a reason for hiding this comment

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

I immediately saw this, thought 🤔 "y no switch", and then saw how annoying this is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, unfortunately the API isn't too friendly :)

Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

LGTM, some comments.

@mtojek mtojek merged commit cb77f04 into main Jan 12, 2024
@mtojek mtojek deleted the 8501-parse-tfvars-3 branch January 12, 2024 14:08
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: cli/api: terraform.tfvars and *.auto.tfvars are silently ignored
2 participants