Skip to content

feat: enable Helm charts installation from flux sources #1350

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

Merged
merged 9 commits into from
Apr 8, 2025

Conversation

BROngineer
Copy link
Contributor

@BROngineer BROngineer commented Apr 3, 2025

This PR adds functionality to install apps defined in ServiceTemplates using Helm charts from flux sources.

Fixes: #1253

Example:

# ServiceTemplate

apiVersion: k0rdent.mirantis.com/v1alpha1
kind: ServiceTemplate
metadata:
  name: prometheus-27-8-0
  namespace: kcm-system
spec:
  helm:
    chartSource:
      path: "./charts/prometheus/"
      remoteSourceSpec:
        git:
          url: "https://github.com/prometheus-community/helm-charts.git"
          ref:
            tag: "prometheus-27.8.0"
# MultiClusterService

apiVersion: k0rdent.mirantis.com/v1alpha1
kind: MultiClusterService
metadata:
  name: global-services
  namespace: kcm-system
spec:
  clusterSelector:
    matchLabels:
      feature.k0rdent.mirantis.com/helm-sources: "true"
  serviceSpec:
    services:
      - template: prometheus-27-8-0
        name: managed-prometheus
        namespace: default
        values: |
          alertmanager:
            enabled: false
          prometheus-pushgateway:
            enabled: false

Results in ClusterProfile with following spec:

apiVersion: config.projectsveltos.io/v1beta1
kind: ClusterProfile
metadata:
  name:
spec:
  clusterSelector:
    matchLabels:
      feature.k0rdent.mirantis.com/helm-sources: "true"
  continueOnConflict: true
  continueOnError: false
  helmCharts:
  - helmChartAction: Install
    releaseName: managed-prometheus
    releaseNamespace: default
    repositoryURL: GitRepository://kcm-system/prometheus-27-8-0/charts/prometheus/
    values: |
      alertmanager:
        enabled: false
      prometheus-pushgateway:
        enabled: false
  reloader: false
  stopMatchingBehavior: WithdrawPolicies
  syncMode: Continuous
  tier: 2147483547

where repositoryURL is a combination of <flux-source-kind>://<repository-namespace>/<repository-name>/<path-defined-in-template>.

ClusterSummary status:

status:
  dependencies: no dependencies
  featureSummaries:
  - featureID: Helm
    hash: FAQ0kOxqL2tNzKp4v2/97PXFY4xg+2mKQWO82sm8myI=
    lastAppliedTime: "2025-04-03T12:05:35Z"
    status: Provisioned
  helmReleaseSummaries:
  - releaseName: managed-prometheus
    releaseNamespace: default
    status: Managing
    valuesHash: xfvq+CF+WTPp9+ftyaRtiOhNQOefJLquRRYBTA/+KyA=

And deployed service:

❯ KUBECONFIG=$(pwd)/aws-kconfig k get pods
NAME                                        READY   STATUS    RESTARTS   AGE
managed-prometheus-server-bdbbdd97d-47xt8   2/2     Running   0          28m

@BROngineer BROngineer force-pushed the enh/helm-sources branch 2 times, most recently from f478e05 to 85dff5b Compare April 3, 2025 16:04
Copy link

github-actions bot commented Apr 7, 2025

Review file api/v1alpha1/clustertemplate_types.go
The code is well-structured and follows Kubernetes best practices but needs improvements in validation, error handling, and clarity. Key areas for enhancement include validating both annotations and spec versions, improving error messages for better debugging, renaming a confusing struct field, adding comprehensive unit tests, and enhancing documentation. The final score is 8/10, recognizing good practices but emphasizing the need for thorough validation and testing to ensure reliability and maintainability.
Review file api/v1alpha1/providertemplate_types.go
The code defines a Kubernetes API resource, ProviderTemplate, correctly structured with proper validation and efficient methods. It adheres to Kubernetes conventions but has potential nil pointer and validation issues. Documentation, error handling, and testing need improvement. The final score is 8, indicating good quality with room for enhancement.
Review file api/v1alpha1/servicetemplate_types.go
The code defines a Kubernetes API for a ServiceTemplate resource, ensuring correctness through validation and mutual exclusivity checks. It efficiently uses struct embedding and follows Kubernetes API patterns. However, potential issues include missing validation in FillStatusWithProviders, nil pointer risks, and a lack of inline comments and tests. The code is well-structured but could benefit from improved error handling and readability. Final score: 8.5/10.
Review file api/v1alpha1/templates_common.go
The code is well-structured, follows Go best practices, and efficiently handles various edge cases. It correctly processes HelmSpec and annotations with proper error handling and validation. The code is clean, readable, and uses descriptive names and comments for clarity. It leverages constants, helper functions, and efficient operations to minimize allocations and improve performance. The final score of 9 reflects its high quality and adherence to best practices.
Review file api/v1alpha1/zz_generated.deepcopy.go
The code implements deep copy functions for Kubernetes API objects, ensuring correct handling of nested structures. It follows standard patterns for DeepCopy methods but has areas for efficiency improvement, such as redundant copying of simple types. Readability is good despite repetitive functions, and the code adheres to Kubernetes best practices. Minor potential bugs include nil pointer dereferences and slice copying issues. Overall, the code is solid with a rating of 9/10.
Review file internal/controller/servicetemplate_controller.go
The Kubernetes controller for ServiceTemplate is well-structured and functional, handling various source types efficiently. However, it has areas for improvement: 1. Redundancy Reduction: The code repeats logic across different source types. Extracting this into helper functions would streamline the code and adhere to the DRY principle. 2. Error Handling Consistency: Implementing a consistent error format, such as using errors.Wrap, would enhance debugging by providing detailed context. 3. Clarity in Logic: Simplifying the switch statement, perhaps by using helper functions, would make the code clearer and reduce complexity. 4. Enhanced Logging: Adding detailed logs would aid in understanding the reconciliation flow and debugging issues. 5. Struct for Source Handling: Encapsulating source logic in a struct could unify processing, reducing the need for repetitive if-else statements. These improvements would enhance maintainability, readability, and scalability, making the code more robust and easier to manage.
Review file internal/sveltos/profile.go
The code review highlights a well-structured Go application that effectively handles Kubernetes API interactions, ensuring correctness and proper error handling. While efficient in using batch operations, it could benefit from improved efficiency by reducing individual API calls. Readability can be enhanced by breaking down lengthy functions and adding detailed comments. Adherence to best practices, such as using context.CancelFunc, is commendable. Potential improvements include adding debug logs, comprehensive error messages, validation checks, and unit tests for critical functions. Overall, the code scores an 8/10, indicating room for enhancement in maintainability and performance.See the code review here

Copy link

github-actions bot commented Apr 7, 2025

Review file api/v1alpha1/clustertemplate_types.go
The code implements a Kubernetes API resource for ClusterTemplate with proper structs, error handling, and validations. It adheres to Kubernetes best practices but lacks documentation and error wrapping. The use of %w without error wrapping is a notable issue. Suggestions include adding comprehensive documentation and ensuring helper functions are correctly implemented. Overall, the code is well-structured with minor improvements needed, earning a final score of 9.
Review file api/v1alpha1/providertemplate_types.go
The code is well-structured and follows Go best practices for Kubernetes API definitions. It uses appropriate struct embedding and JSON tags, and the functions are correctly implemented. However, there are areas for improvement, including adding more descriptive comments and examples, improving error messages in FillStatusWithProviders, and adding Godoc comments to functions. Final score: 8. <final_score>8</final_score>
Review file api/v1alpha1/servicetemplate_types.go
The code implements a Kubernetes API for ServiceTemplate with proper validations and embedded specs, ensuring correct and efficient handling. However, it lacks comments and has unclear function names, affecting readability. It adheres to Kubernetes best practices but has potential bugs like nil pointer access and incomplete validation. Areas for improvement include simplifying conditionals, adding nil checks, enhancing error messages, and expanding validation coverage. Overall, the code is well-structured but needs refinement to enhance robustness and maintainability, earning an 8/10 score.
Review file api/v1alpha1/templates_common.go
The code is well-structured but has critical issues: case-sensitive validation, nil pointer dereference, lack of documentation, unclear error messages, and potential conflicts between spec and annotations. Suggested fixes include adding unit tests, improving variable names, and clarifying complex logic. Overall, the code is solid but needs improvements for robustness and readability, earning an 8/10 rating.
Review file api/v1alpha1/zz_generated.deepcopy.go
The generated code correctly implements deep copy functions for Kubernetes API objects, ensuring all fields and nested structures are properly copied. It efficiently handles pointers, slices, and maps, preventing shared references and panics. While readable due to consistent patterns, it lacks comments, which could improve maintainability. Adhering to best practices, it could benefit from optimizations for large data structures and better documentation. Overall, the code is well-structured and suitable for Kubernetes use with a final score of 9.
Review file internal/controller/servicetemplate_controller.go
The code implements a ServiceTemplate controller with some critical issues: bugs in condition handling and status updates, potential unnecessary requeues, and inefficiencies in label handling. Logging is clear, function names are mostly descriptive, and ownership/predicates follow Kubernetes best practices. However, comments are lacking in complex logic, and condition copying needs improvement. The final score is 8/10, with recommendations to fix bugs, optimize requeues, and handle all source types properly.
Review file internal/sveltos/profile.go
The code is well-structured and adheres to best practices, with efficient use of resources and consistent logging. Strengths include correct error handling and proper object references. Areas for improvement include enhancing error messages, adding logging for debugging, and checking context cancellation. Redundant code and lack of documentation in some functions are noted. The final score of 9 reflects the code's potential for robustness with suggested enhancements.See the code review here

Copy link

github-actions bot commented Apr 7, 2025

Review file api/v1alpha1/clustertemplate_types.go
The code is well-structured but needs improvements in error handling, documentation, variable naming, validation, logging, readability, testing, and pointer usage. Key points include replacing %w in error messages, adding function comments, renaming variables for clarity, enhancing validation, adding logging for debugging, breaking down functions, documenting types, considering pointer returns, fixing error propagation, and adding unit tests. The final score of 8/10 reflects the code's good structure but highlights areas for better maintainability and clarity.
Review file api/v1alpha1/providertemplate_types.go
The code is well-structured and follows Kubernetes API conventions, with clear struct definitions, efficient implementation, and good readability. It adheres to best practices, including the use of metav1 types and validation annotations. However, there are minor areas for improvement, such as simplifying error handling in FillStatusWithProviders and adding more comments to explain method purposes. Returning copies instead of pointers in GetHelmSpec and GetCommonStatus could also enhance safety. Overall, the code is solid and effectively implements the required functionality, earning a rating of 8/10.
Review file api/v1alpha1/servicetemplate_types.go
The code review evaluates a Kubernetes API for a ServiceTemplate resource, noting strengths in struct definitions, validation, and efficiency but highlighting the need for better documentation, error handling, and testing. It commends the use of inline embedding and adherence to best practices while suggesting improvements in function readability and field naming. The review concludes the code is correct but could be more robust and maintainable with suggested enhancements.
Review file api/v1alpha1/templates_common.go
The code review highlights issues in the Kubernetes API package for Helm charts, including validation rules needing improvement, lack of documentation, and complex functions that could be refactored. The HelmSpec struct enforces mutual exclusivity but not exactly one field being set. Functions like getCAPIContracts are complex and benefit from smaller helper functions. Error handling is good but needs Go 1.16+ compatibility, and variable naming like merr should be standardized. Adding documentation and addressing edge cases would enhance readability and maintainability. The code is well-structured but could be improved for clarity and robustness.
Review file api/v1alpha1/zz_generated.deepcopy.go
The code provides efficient and correct deep copy functions for Kubernetes CRDs, generated by tools like controller-gen, ensuring no shared references between objects. It handles nested structs, slices, and maps effectively, using pre-allocated resources and checking for non-nil fields. While there are minor issues like redundant nil checks and inefficient copying of primitives, the code adheres to best practices and is well-written, earning a final rating of 9/10.
Review file internal/controller/servicetemplate_controller.go
The code review highlights the strengths of a Kubernetes controller for ServiceTemplates, including clear structure, error handling, and resource ownership, while pointing out areas for improvement such as inconsistent logging, a potential bug in condition checks, and the lack of finalizers for resource cleanup. Suggestions include standardizing logging, fixing the condition check, adding finalizers, reducing code redundancy, and improving error logging. The code is well-structured but requires fixes to enhance reliability and maintainability. Final score: 8/10.
Review file internal/sveltos/profile.go
The code demonstrates strong adherence to Kubernetes best practices and handles various functionalities such as profile reconciliation, Helm chart management, and Kustomization. However, there are opportunities to improve efficiency, readability, and error handling. The code's structure is well-organized, but certain areas could benefit from more concise logic and better documentation. Overall, the implementation shows a solid understanding of Kubernetes controller patterns, but minor refinements would enhance maintainability. The final score reflects the code's strengths and areas for improvement, landing at 8.5. xml <final_score>8.5</final_score> See the code review here

Copy link

github-actions bot commented Apr 7, 2025

Review file api/v1alpha1/clustertemplate_types.go
The code defines a Kubernetes ClusterTemplate resource with a structured API but lacks validation for some fields. Potential issues include undefined functions and incomplete error handling. It's efficient but could improve by validating versions earlier. Readability is good, though more documentation would help. The code follows best practices but needs better validation, error messages, and function implementations for robustness. Final score: 8/10.
Review file api/v1alpha1/providertemplate_types.go
The code defines a Kubernetes API resource for ProviderTemplate with structs and helper functions. It has issues like a missing field in Status, unimplemented helper functions, and lack of Helm chart validation. The code is efficient and readable but could benefit from better documentation and validation. It adheres to best practices but needs logging, error handling, and unit tests. The final score is 8, indicating it's well-structured but requires improvements.
Review file api/v1alpha1/servicetemplate_types.go
The code defines a Kubernetes ServiceTemplate resource with proper API conventions and validation. It uses pointers efficiently and has clear struct definitions. Areas for improvement include handling empty constraints, nil checks, and label consistency. The final score is 8 out of 10.
Review file api/v1alpha1/templates_common.go
The Go code is well-structured and demonstrates good practices, including proper struct definitions, validation rules, and efficient string operations. Areas for improvement include adding more detailed comments, breaking down complex functions into smaller helpers, and using early returns to enhance readability and efficiency. Error handling is appropriate but could benefit from more detailed messages. The code adheres to best practices with constants and necessary imports, ensuring maintainability. Overall, the code is effective and scores a 9 out of 10 with minor enhancements needed.
Review file api/v1alpha1/zz_generated.deepcopy.go
The code review evaluates generated deepcopy functions for Kubernetes API objects, focusing on correctness, efficiency, readability, and best practices. The functions handle nested structs, slices, and nil pointers correctly, with consistent structure and clear variable names. While efficient for its purpose, improvements like adding comments and simplifying some logic could enhance readability. No critical issues were found, and the code is well-structured and maintainable. Overall, it's a solid implementation with room for minor enhancements.
Review file internal/controller/servicetemplate_controller.go
The code review identifies issues such as a condition check using a bitwise AND instead of a logical AND, inconsistent error handling, and incomplete status updates. It suggests improving efficiency by using slice operations, enhancing readability with clearer comments, and adhering to best practices by handling the ctx.Done() channel. Additionally, it recommends adding object type validation for security and ensuring consistent error wrapping. The final rating is 9, indicating the code is solid but requires minor fixes for robustness and maintainability. 1. Correctness Issues: The code has a condition check bug using a bitwise AND, inconsistent error handling, and incomplete status updates. 2. Efficiency and Readability: Improvements are suggested for using slice operations and adding clearer comments. 3. Best Practices and Security: Handling ctx.Done() and object type validation are recommended. 4. Areas for Improvement: Consistent error wrapping, comprehensive status updates, and better documentation. 5. Final Rating: The code scores a 9, indicating it is solid but needs minor fixes for better maintainability.
Review file internal/sveltos/profile.go
The code is well-structured and follows Go best practices but has areas for improvement. Key issues include potential incorrect priority-to-tier mapping and manual patch construction, which could lead to errors. Efficiency could be enhanced by optimizing object fetching and reducing redundancy through helper functions. Readability and maintainability would benefit from clearer variable names, better documentation, and improved error handling. The final score is 8, suggesting the code is good but could be more robust and maintainable with suggested changes.See the code review here

@BROngineer BROngineer requested a review from wahabmk April 7, 2025 20:54
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Signed-off-by: Artem Bortnikov <[email protected]>
Copy link

github-actions bot commented Apr 8, 2025

Review file api/v1alpha1/clustertemplate_types.go
The code is well-structured and adheres to Kubernetes API conventions, with proper implementation of status field population and getter methods. It efficiently uses data structures and maintains readability with clear struct definitions and JSON tags. Error handling follows best practices, but there are potential issues like missing null checks for annotations, insufficient documentation for helper functions, and a lack of unit tests. Overall, the code achieves a solid 8/10 rating, with minor improvements needed for robustness and documentation.
Review file api/v1alpha1/providertemplate_types.go
The code implements a Kubernetes API resource ProviderTemplate with correct structure and validation, adhering to Kubernetes conventions. It includes proper error handling and immutability enforcement but lacks documentation and has minor issues with error formatting. The FillStatusWithProviders function processes data efficiently but relies on external functions and lacks input validation. The code scores 8/10, with recommendations for improved documentation, standardized error handling, and additional testing.
Review file api/v1alpha1/servicetemplate_types.go
The code defines a Kubernetes API resource for deploying applications using Helm, Kustomize, or raw resources, with validation ensuring mutual exclusivity of deployment strategies. It includes status structures and utility functions but has issues with redundant checks, readability, and missing input validation. Areas for improvement include adding documentation, simplifying code, and enhancing error handling. The final score is 8/10. --- Summary: - The code introduces a Kubernetes API resource for deploying applications using Helm, Kustomize, or raw resources, ensuring mutual exclusivity of deployment strategies through validation. - It includes well-structured status definitions but has efficiency issues like redundant checks and readability problems due to unclear function names and missing comments. - Potential bugs and security vulnerabilities exist, such as missing input validation for certain fields. - Improvements needed include adding documentation, enhancing error logging, simplifying code, and validating input fields. - The code is rated 8/10, acknowledging its structure while highlighting areas for refinement. <final_score>8</final_score>
Review file api/v1alpha1/templates_common.go
The code is logically correct, adheres to best practices, and handles validation rules effectively. Potential improvements include enhancing documentation, optimizing string operations, and improving error messages for better context. Readability can be improved with clearer variable names and function comments. A minor bug exists with empty provider contracts, but there are no security concerns. The code structure is solid, and the final score is 8/10. 1. The code is logically correct and adheres to best practices, including proper validation rules. 2. Areas for improvement include enhancing documentation, optimizing string operations, and improving error messages. 3. Readability can be improved with clearer variable names and function comments. 4. A minor bug exists with empty provider contracts, but there are no security concerns. 5. The code structure is solid, and the final score is 8/10.
Review file api/v1alpha1/zz_generated.deepcopy.go
The code under review implements deepcopy functions for Kubernetes CRDs, ensuring proper object copying. While it correctly handles most fields and pointers, it lacks essential null checks, risking panics. The use of manual loops instead of range loops could be optimized for efficiency and readability. Additionally, the code would benefit from more comments to enhance clarity, especially for those unfamiliar with Kubernetes conventions. Key improvements include adding null checks to prevent nil pointer dereferences, optimizing loops for better performance, and enhancing readability with comments. Despite these shortcomings, the code is functional and scores an 8, indicating its adherence to conventions with room for enhancement. In summary, the code is effective but requires adjustments to improve safety, efficiency, and maintainability.
Review file internal/controller/servicetemplate_controller.go
The Kubernetes controller efficiently reconciles ServiceTemplate objects, handling various sources like Helm charts and remote URLs, with proper status updates using defer. It ensures correct management object checks and efficient reconciliation via predicates. However, areas for improvement include adding a break in the Helm switch case, removing unnecessary requeues, enhancing error messages, and using more descriptive variable names. Overall, the code is well-structured with a final score of 9/10, reflecting its strong foundation and minor enhancement opportunities.
Review file internal/sveltos/profile.go
The code demonstrates a solid implementation of Kubernetes controller logic for managing Sveltos profiles but has areas for improvement. Potential issues include incorrect Helm chart URLs and unexpected behavior in priority-to-tier conversion. Efficiency can be enhanced with caching and bulk API operations, while readability could be improved with clearer variable names and additional comments. Best practices could be strengthened with better input validation and reduced code redundancy. Overall, the code is functional but needs minor adjustments for efficiency, readability, and security. Final score: 8.See the code review here

@BROngineer BROngineer merged commit 34fb266 into k0rdent:main Apr 8, 2025
7 checks passed
@github-project-automation github-project-automation bot moved this to Done in k0rdent Apr 8, 2025
@BROngineer BROngineer deleted the enh/helm-sources branch April 8, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[enhancement] allow installing Helm charts from flux sources
3 participants