Parse terraform local module source #3461
Conversation
|
This pull request is NOT APPROVED. It will be approved when one of the following conditions is met:
|
|
Code coverage for golang is
|
| func isModuleLocal(moduleSrc string) bool { | ||
| if strings.HasPrefix(moduleSrc, "./") || strings.HasPrefix(moduleSrc, "../") { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } |
There was a problem hiding this comment.
nits
| func isModuleLocal(moduleSrc string) bool { | |
| if strings.HasPrefix(moduleSrc, "./") || strings.HasPrefix(moduleSrc, "../") { | |
| return true | |
| } | |
| return false | |
| } | |
| func isLocalModule(src string) bool { | |
| return strings.HasPrefix(src, "./") || strings.HasPrefix(src, "../") | |
| } |
There was a problem hiding this comment.
thank you for the suggestion!
|
Code coverage for golang is
|
| for _, f := range files { | ||
| for _, m := range f.Modules { | ||
| if m.IsLocal { | ||
| l := provider.NewLocalModuleSourceConverter(in.GitPath.Repo.Remote, in.GitPath.Repo.Branch, ds.RepoDir, ds.AppDir) | ||
| url, err := l.MakeURL(m.Source) | ||
| if err != nil { | ||
| return out, err | ||
| } | ||
|
|
||
| m.Source = url | ||
| } else { | ||
| // TODO: convert remote module to URL. | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This shouldn't be included in this function. I think we should pass a GitPath to FindArtifactVersions to handle inside that function, instead.
There was a problem hiding this comment.
I see I'll fix it! Thank you!
ds.RepoDir, ds.AppDir are also needed, so I'll fix to pass a GitPath and a ds!
|
The following ISSUES will be created once got merged. If you want me to skip creating the issue, you can use Details1. convert remote module to URL.pipecd/pkg/app/piped/cloudprovider/terraform/module.go Lines 118 to 121 in 4dbe548 This was created by todo plugin since "TODO:" was found in 4dbe548 when #3461 was merged. cc: @ffjlabo. |
|
Code coverage for golang is
|
|
Nice |
| // FindArtifactVersions parses artifact versions from Terraform files. | ||
| // For Terraform, module version is an artifact version. | ||
| func FindArtifactVersions(tfs []File) ([]*model.ArtifactVersion, error) { | ||
| func FindArtifactVersions(tfs []File, gp *model.ApplicationGitPath, ds *deploysource.DeploySource) ([]*model.ArtifactVersion, error) { |
There was a problem hiding this comment.
model.ApplicationGitPath can be passed directly by value instead of a pointer.
There was a problem hiding this comment.
Only passing the needed fields instead of all deploysource.DeploySource object.
| RepoDir: repoDir, | ||
| AppDir: AppDir, |
There was a problem hiding this comment.
Maybe we don't need these fields.
The File struct can contain a relative path of itself from the root of the repository.
|
@ffjlabo Thanks for your great work! Please feel free to transfer this PR to me. 👍 /hold |
|
/hold |
|
Please create another PR when you'll work on this task. |
|
CLA Assistant Lite bot: Welcome! /cla sign You can retrigger this bot by commenting recheck in this Pull Request |
What this PR does / why we need it:
This PR changes to show URL for terraform local module.
ref: local module https://www.terraform.io/language/modules/sources#local-paths
Which issue(s) this PR fixes:
A part of #3303
Does this PR introduce a user-facing change?: