Skip to content
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

Jump to bottom

Feat: add new option to template which populates variables from sourc… #393

Merged
merged 4 commits into from
Jun 8, 2022
Merged

Feat: add new option to template which populates variables from sourc… #393

merged 4 commits into from
Jun 8, 2022

Conversation

TomerHeber
Copy link
Collaborator

@TomerHeber TomerHeber commented May 24, 2022 •
edited
Loading

…e code

Issue & Steps to Reproduce / Feature Request

resolves #359

Solution

Some of the code was split to #410

  1. Added a new data source.
  2. Added a new API call.
  3. Added a new writeResourceData type for Slices.
  4. Modified integration tests.
  5. Added acceptance tests.
  6. Added unit tests.
  7. Updated DEVELOPMENT.md with new util functions.
  8. Added examples.

@TomerHeber TomerHeber self-assigned this May 24, 2022
TomerHeber
TomerHeber commented May 24, 2022
View reviewed changes
env0/data_source_code_variables.go
Type: schema.TypeList,
Description: "a list of terraform variables extracted from the source code",
Computed: true,
Elem: &schema.Resource{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think these 3 will suffice.
Let me know if I should add any additional fields.

TomerHeber
TomerHeber commented May 24, 2022
View reviewed changes
env0/data_source_code_variables.go
ReadContext: dataSourceCodeVariablesRead,

Schema: map[string]*schema.Schema{
"template_id": {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm assuming VCS is configured in a template (or environment with an implicit template).
Hopefully this assumption is correct.

TomerHeber
TomerHeber commented May 24, 2022
View reviewed changes
env0/data_source_code_variables.go
return diag.Errorf("failed to extract variables from repository: %v", err)
}

if err := writeResourceDataSlice(variables, "variables", d); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some "magic" here.
Basically, this is a version of "writeResourceData" for slices.

TomerHeber
TomerHeber commented May 24, 2022
View reviewed changes
env0/utils.go Outdated Show resolved Hide resolved
TomerHeber
TomerHeber commented May 24, 2022
View reviewed changes
env0/utils.go Outdated

// Extracts values from a slice of interfaces, and writes it to resourcedata at name.
func writeResourceDataSlice(i interface{}, name string, d *schema.ResourceData) error {
writeResourceDataSliceStruct := func(val reflect.Value, value map[string]interface{}) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This add supports structs within structs. Hopefully, this is enough to cover the vast majority of use cases.

@TomerHeber TomerHeber requested a review from yaronya May 24, 2022 20:26
sagilaufer1992
sagilaufer1992 reviewed May 29, 2022
View reviewed changes
Copy link
Contributor

@sagilaufer1992 sagilaufer1992 left a comment

Choose a reason for hiding this comment

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

Didn't review all, but left some comments for now

DEVELOPMENT.md Outdated Show resolved Hide resolved
client/template.go Show resolved Hide resolved
@TomerHeber TomerHeber force-pushed the feat-variables-from-source-code-#359 branch from c09311e to 4646791 Compare May 29, 2022 13:47
@TomerHeber TomerHeber mentioned this pull request Jun 4, 2022
Merged
@sagilaufer1992 sagilaufer1992 removed their request for review June 6, 2022 10:23
liranfarage89
liranfarage89 reviewed Jun 6, 2022
View reviewed changes
Copy link
Contributor

@liranfarage89 liranfarage89 left a comment

Choose a reason for hiding this comment

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

Great work! ( like a pro )
Left some questions and I think you missed description & is_sensitive

env0/data_source_code_variables.go Show resolved Hide resolved
env0/data_source_code_variables.go Show resolved Hide resolved
env0/data_source_code_variables.go Show resolved Hide resolved
env0/utils.go Outdated Show resolved Hide resolved
@TomerHeber
Copy link
Collaborator Author

@liranfarage89 - Thank you for reviewing so far.

let's put on hold the PR, I want to reduce the size so it will be easier to review. Will update you once it's ready for review again.

@TomerHeber TomerHeber force-pushed the feat-variables-from-source-code-#359 branch from 89fc8d8 to 956631f Compare June 7, 2022 14:33
TomerHeber
TomerHeber commented Jun 7, 2022
View reviewed changes
env0/utils.go
@@ -19,6 +19,10 @@ type CustomResourceDataField interface {
WriteResourceData(fieldName string, d *schema.ResourceData) error
}

type ResourceDataSliceStructValueWriter interface {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This interface is for custom writing.
see func (c *ConfigurationVariableSchema) ResourceDataSliceStructValueWrite(values map[string]interface{}) error {

Unit test is below.

@TomerHeber TomerHeber force-pushed the feat-variables-from-source-code-#359 branch 3 times, most recently from 107fc84 to 092c2e4 Compare June 7, 2022 18:43
@TomerHeber TomerHeber force-pushed the feat-variables-from-source-code-#359 branch from 9d26103 to 9bf84cf Compare June 7, 2022 18:49
@TomerHeber
Copy link
Collaborator Author

@liranfarage89 - Thank you for reviewing so far.

let's put on hold the PR, I want to reduce the size so it will be easier to review. Will update you once it's ready for review again.

@liranfarage89 - you may continue reviewing. The PR size was reduced and updated the code based on your comments.

liranfarage89
liranfarage89 approved these changes Jun 8, 2022
View reviewed changes
Copy link
Contributor

@liranfarage89 liranfarage89 left a comment

Choose a reason for hiding this comment

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

You rock 🎸 🚀 @TomerHeber 🙇

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Jun 8, 2022
@TomerHeber TomerHeber merged commit f1a1a2b into main Jun 8, 2022
@TomerHeber TomerHeber deleted the feat-variables-from-source-code-#359 branch June 8, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagilaufer1992 sagilaufer1992 sagilaufer1992 left review comments

@liranfarage89 liranfarage89 liranfarage89 approved these changes

@yaronya yaronya Awaiting requested review from yaronya

Assignees

@TomerHeber TomerHeber

Labels
api-client examples feature integration-tests provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add new option to template which populates variables from source code
3 participants
@TomerHeber @sagilaufer1992 @liranfarage89