Support K8sApp by pipectl init in simplest way
#4759
Conversation
Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: t-kikuc <[email protected]>
t-kikuc
requested review from
nghialv,
khanhtc1202,
kentakozuka and
ffjlabo
as code owners
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4759 +/- ##
==========================================
+ Coverage 31.02% 31.39% +0.37%
==========================================
Files 225 226 +1
Lines 26257 26521 +264
==========================================
+ Hits 8146 8327 +181
- Misses 17460 17534 +74
- Partials 651 660 +9 ☔ View full report in Codecov by Sentry. |
Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: t-kikuc <[email protected]>
|
/review |
PR AnalysisMain themeEnhancement PR summaryThis PR adds functionality to generate configuration for Kubernetes-based applications in the PipeCD project initialization command ( Type of PREnhancement PR Feedback:General suggestionsThe implementation of Kubernetes configuration generation seems to cover both Helm and Kustomize users, which is comprehensive and user-friendly. By prompting users for specific details, it avoids potential misconfigurations and simplifies the initial setup process. The code looks clean and maintains consistent formatting and encapsulation. Code feedback
Security concerns:no The added functionality does not seem to introduce any direct security concerns. User inputs are used to generate configuration, which is less likely to be a security threat in the context of initial setup scripts. However, it is always good to validate and sanitize any user inputs to prevent potential security issues in the future. |
Signed-off-by: t-kikuc <[email protected]>
| return nil, err | ||
| } | ||
|
|
||
| if len(chartRepository) > 0 { |
There was a problem hiding this comment.
| if len(chartRepository) > 0 { | |
| if chartRepository != "" { |
| if len(chartRepository) > 0 { | ||
| // Use remote chart | ||
| err = p.RunSlice(remoteChartInputs) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } else { | ||
| // Use local chart | ||
| err = p.RunSlice(localChartInputs) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
[nit] It would be nice to define the remoteChartInputs and localChartInputs in each if statement to minimize their scope. 👀
| inputs := []prompt.Input{ | ||
| { | ||
| Message: "Kustomize version", | ||
| TargetPointer: &kustomizeVersion, | ||
| Required: false, | ||
| }, | ||
| } | ||
|
|
||
| err := p.RunSlice(inputs) |
There was a problem hiding this comment.
| inputs := []prompt.Input{ | |
| { | |
| Message: "Kustomize version", | |
| TargetPointer: &kustomizeVersion, | |
| Required: false, | |
| }, | |
| } | |
| err := p.RunSlice(inputs) | |
| input := prompt.Input{ | |
| Message: "Kustomize version", | |
| TargetPointer: &kustomizeVersion, | |
| Required: false, | |
| } | |
| err := p.Run(input) |
| case helm: | ||
| deploymentInput, err = helmInput(p) | ||
| default: | ||
| return nil, errors.New("invalid number") |
There was a problem hiding this comment.
| return nil, errors.New("invalid number") | |
| return nil, fmt.Errorf("invalid manifest manager: %s", manifestManager) |
Signed-off-by: t-kikuc <[email protected]>
|
@ffjlabo |
|
@khanhtc1202 Would you check this? (not in a hurry) |
What this PR does / why we need it:
Support generating the simplest app.pipecd.yaml for K8sApp by
pipectl init, selecting Kustomize or Helm.
Which issue(s) this PR fixes:
Fixes #4753
Does this PR introduce a user-facing change?: no