-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat: introduce json format for inline module (#165) #283
feat: introduce json format for inline module (#165) #283
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I think it looks fine - a couple of comments inline. Also maybe add an example file to document what the JSON format looks like in the inline module. Thanks!
@@ -249,8 +250,12 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E | |||
} | |||
|
|||
case v1beta1.ModuleSourceInline: | |||
if err := c.fs.WriteFile(filepath.Join(dir, tfMain), []byte(cr.Spec.ForProvider.Module), 0600); err != nil { | |||
return nil, errors.Wrap(err, errWriteMain) | |||
filename := tfMain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think we prefer short variable names, so maybe fn
or f
instead of filename
Signed-off-by: Dennis Kniep <[email protected]>
588d051
to
db0eb9e
Compare
@bobh66 addressed your review comments |
@bobh66 can we merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thhanks @denniskniep - LGTM. It would be nice to have an example Workspace
that shows the JSON format, but presumably anyone interested in using the JSON format already knows what it looks like.
Description of your changes
API Change:
Added a new property
InlineFormat
toWorkspaceParameters
which specifies the format of the inline Terraform content. It could beHCL
orJSON
Change:
If
InlineFormat
is set toJSON
the file which is created is namedmain.tf.json
instead ofmain.tf
Fixes #165
I have:
make reviewable
to ensure this PR is ready for review.How has this code been tested
Wrote a UnitTest and already tested it in my environment