Support K8sApp by `pipectl init` in simplest way
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
- How are users affected by this change:
- Is this breaking change:
- How to migrate (if breaking change):
Codecov Report
Attention: 24 lines in your changes are missing coverage. Please review.
Comparison is base (
7b26b02) 31.02% compared to head (a0e9045) 31.39%. Report is 16 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| pkg/app/pipectl/cmd/initialize/kubernetes.go | 86.30% | 16 Missing and 7 partials :warning: |
| pkg/app/pipectl/cmd/initialize/initialize.go | 0.00% | 1 Missing :warning: |
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
/review
PR Analysis
Main theme
Enhancement
PR summary
This PR adds functionality to generate configuration for Kubernetes-based applications in the PipeCD project initialization command (pipectl init). Specifically, it allows the user to choose whether their application will manage its manifests using Kustomize or Helm and then prompts the user to enter the details needed for the chosen method, including support for a remote Helm chart or a local chart.
Type of PR
Enhancement
PR Feedback:
General suggestions
The 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
-
relevant file:
pkg/app/pipectl/cmd/initialize/kubernetes.gosuggestion: Consider validation for user input during the configuration process. For strings that represent URLs, versions, or paths, ensuring that they are well-formed before proceeding can mitigate runtime issues. For instance, adding checks to ensure thatchartRepositoryis a valid URL orhelmVersionmatches expected version patterns would be beneficial. (medium) relevant line:+ chartRepository string -
relevant file:
pkg/app/pipectl/cmd/initialize/kubernetes.gosuggestion: Typos in method names can make it difficult for others to understand and maintain the code. Correct the typo inhelmImputtohelmInput. This is a simple but important fix for code maintainability. (important) relevant line:+func helmImput(p prompt.Prompt) (*config.KubernetesDeploymentInput, error) { -
relevant file:
pkg/app/pipectl/cmd/initialize/initialize.gosuggestion: Removing the panic and providing an implementation for the Kubernetes platform is good. However, it might be beneficial to check whethererris notnilafter callinggenerateKubernetesConfig(p)and handle the error accordingly before proceeding. (medium) relevant line:+ cfg, err = generateKubernetesConfig(p)
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.
@ffjlabo Thank you so much! I fixed all above.
@khanhtc1202 Would you check this? (not in a hurry)