feat: improve helm and kubeconfig packages

[juev]

Overview

Improve code

What this PR does / why we need it

This pull request includes significant changes across multiple files, focusing on improving thread safety, enhancing utility functions, and refining logging. Below is a categorized summary of the most important changes:

Thread Safety Enhancements:

• Updated TaskHandleHistory in pkg/addon-operator/operator_test.go to use a thread-safe structure with a mutex for managing task history. This ensures safe concurrent access in test cases. [1] [2] [3]
• Refactored Checksums in pkg/kube_config_manager/checksums.go to be thread-safe by introducing a sync.RWMutex and encapsulating unsafe operations in helper methods. [1] [2]

Utility Function Improvements:

• Introduced a new parseSetValues function in pkg/helm/helm3lib/helm3lib.go to parse Helm setValues with better handling of edge cases, such as trimming whitespace and supporting values with multiple = characters. [1] [2] [3]
• Added comprehensive unit tests for parseSetValues to validate its behavior across various scenarios.

Logging Refinements:

• Replaced direct log calls with the logger instance in pkg/helm_resources_manager/helm_resources_manager.go to standardize logging and improve context. [1] [2] [3]

Code Simplifications:

• Used the maps.Copy function from the Go standard library in pkg/helm/helm3lib/helm3lib.go and pkg/helm/post_renderer/post_renderer.go to simplify copying map data. [1] [2]

Naming and Readability Improvements:

• Renamed the l field to mu in pkg/helm_resources_manager/helm_resources_manager.go to better reflect its purpose as a mutex.

[Copilot]

Pull Request Overview

This PR improves the helm and kubeconfig packages by refactoring functions to simplify value parsing, enhance error handling, and enforce thread-safety with proper locking. Key changes include:

• Adding utility functions (e.g., parseSetValues) to reduce code duplication.
• Enhancing error propagation and introducing logging improvements.
• Incorporating thread-safety with locking mechanisms and ensuring proper resource cleanup.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added locks in various functions and updated event handling logic.
pkg/kube_config_manager/checksums.go	Inserted nil checks before initializing maps.
pkg/kube_config_manager/backend/configmap/configmap.go	Improved error propagation and ensured proper namespace assignment.
pkg/helm_resources_manager/resources_monitor.go	Added timer cleanup and continue statements to better handle errors.
pkg/helm_resources_manager/helm_resources_manager.go	Updated logging calls and ensured thread-safety in namespace updates.
pkg/helm/post_renderer/post_renderer.go	Utilized maps.Copy for handling extra labels in post-rendering.
pkg/helm/helm3lib/helm3lib.go	Refactored setValues parsing and updated rollback error management.

Comments suppressed due to low confidence (1)

pkg/helm_resources_manager/helm_resources_manager.go:81

• [nitpick] The log message uses 'Helm resourcer manager' which may be a typographical error. Consider changing it to 'Helm resources manager' for clarity.

logger.Debug("Helm resourcer manager: cache has been synced")

[Copilot]

Pull Request Overview

This PR enhances the Helm and kubeconfig packages by refactoring common logic, improving error handling, adding thread-safety, and adopting structured logging.

• Introduced parseSetValues utility and replaced repeated value-parsing loops in Helm client.
• Added mutex locks to critical KubeConfigManager methods and improved error propagation in ConfigMap backend.
• Enhanced resource monitoring by ensuring timer cleanup and skipping iterations on errors.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added locking in IsModuleEnabled/UpdateModuleConfig, inlined module name logic, and adjusted change detection.
pkg/kube_config_manager/checksums.go	Initialized c.sums[name] maps to prevent nil map panics.
pkg/kube_config_manager/backend/configmap/configmap.go	Logged and returned actual errors in mergeValues and set namespace on new ConfigMaps.
pkg/helm_resources_manager/resources_monitor.go	Added defer cleanup for ticker and continue on errors to avoid blocking logic.
pkg/helm_resources_manager/helm_resources_manager.go	Switched to structured logger usage and (over)eagerly canceled cache context.
pkg/helm/post_renderer/post_renderer.go	Used maps.Copy for efficient label merging and safeguarded nil-label maps.
pkg/helm/helm3lib/helm3lib.go	Refactored extra-label merging, added parseSetValues, and surfaced rollback errors.

Comments suppressed due to low confidence (2)

pkg/kube_config_manager/kube_config_manager.go:242

• This method reads and writes kcm.currentConfig.Modules without holding kcm.m.Lock, which can lead to race conditions. Acquire the mutex before modifying shared state.

func (kcm *KubeConfigManager) handleConfigEvent(obj config.Event) {

pkg/helm_resources_manager/resources_monitor.go:95

• Deferring timer.Stop() inside the loop stacks up defer calls until function exit. Consider calling timer.Stop() explicitly when breaking or before continuing to avoid resource leaks.

defer timer.Stop() // Ensure timer is always stopped

[Copilot]

Pull Request Overview

This PR improves the helm and kubeconfig packages by refactoring error handling, enhancing thread-safety, and introducing various utility functions and logging improvements.

• Refactored utility functions (e.g., parseSetValues) to reduce code duplication.
• Enhanced error propagation and logging across multiple packages.
• Introduced locking mechanisms and cleanup improvements for thread-safety and resource management.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added locks and improved module configuration event handling; minor naming suggestion for clarity.
pkg/kube_config_manager/checksums.go	Added nil checks before map allocation in checksum management functions.
pkg/kube_config_manager/backend/configmap/configmap.go	Improved error handling and ensured new ConfigMaps have their namespace set.
pkg/helm_resources_manager/resources_monitor.go	Added cleanup of timers and early continue statements on error handling.
pkg/helm_resources_manager/helm_resources_manager.go	Replaced direct log calls with structured logger calls and added locking for namespace updates.
pkg/helm/post_renderer/post_renderer.go	Adopted maps.Copy for label injection with a nil check to ensure proper label management.
pkg/helm/helm3lib/helm3lib.go	Refactored extra label addition, introduced parseSetValues, and improved rollback error handling.

Comments suppressed due to low confidence (3)

pkg/kube_config_manager/kube_config_manager.go:250

• [nitpick] Consider renaming 'curr' to 'currentModuleCfg' for enhanced clarity when referring to the module configuration.

if curr, ok := kcm.currentConfig.Modules[moduleName]; ok {

pkg/kube_config_manager/backend/configmap/configmap.go:323

• Confirm that setting the namespace here is appropriate for all use cases and consider adding a comment detailing why this specific namespace is used.

obj.Namespace = b.namespace // set namespace for new ConfigMap

pkg/helm/helm3lib/helm3lib.go:282

• Ensure that logging 'SecretsDriverName' reflects the intended deletion operation, as the previous log indicated 'ConfigMapsDriverName'. Verify that the change is deliberate and that it aligns with the actual driver being used.

slog.String("driver", driver.SecretsDriverName))

[Copilot]

Pull Request Overview

This PR improves the helm and kubeconfig packages by enhancing error handling, thread-safety, logging, and code maintainability.

• Introduces utility functions (e.g. parseSetValues) to reduce duplication in setValues parsing.
• Refactors error handling and event processing in kubeconfig management with improved locking and logging.
• Enhances resource monitoring and logging in helm packages with proper timer cleanup and structured logging.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added locking in methods and refactored event handling logic for thread-safety.
pkg/kube_config_manager/checksums.go	Added nil checks and map initialization logic for checksum map management.
pkg/helm_resources_manager/resources_monitor.go	Ensured proper cleanup of timer resources by adding a deferred timer.Stop().
pkg/helm_resources_manager/helm_resources_manager.go	Replaced direct log calls with structured logging and added a lock for namespace updates.
pkg/helm/post_renderer/post_renderer.go	Added a nil-check before merging labels with maps.Copy, ensuring safe label handling.
pkg/helm/helm3lib/helm3lib.go	Consolidated duplicate parsing logic into the new parseSetValues function to improve maintainability.

[Copilot]

Pull Request Overview

This PR improves the code’s maintainability, thread safety, and utility features across the helm and kubeconfig packages. Key changes include refactoring mutex usage to RWMutex with helper functions, standardizing logging via a dedicated logger, and introducing a reusable parseSetValues function to simplify parsing key–value pairs.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Replaced sync.Mutex with sync.RWMutex and updated locking helpers
pkg/kube_config_manager/event_handlers.go	Updated event handler methods with enhanced thread safety and comments
pkg/kube_config_manager/checksums.go	Made the Checksums storage thread-safe with RWMutex and helper methods
pkg/helm_resources_manager/helm_resources_manager.go	Revised locking and logging for resource monitor management
pkg/helm/post_renderer/post_renderer.go	Leveraged new maps.Copy syntax for label merging in post-rendering
pkg/helm/helm3lib/helm3lib.go	Added a robust parseSetValues function to simplify key–value parsing
pkg/helm/helm3lib/helm3lib_test.go	Added comprehensive tests for parseSetValues

[Copilot]

Pull Request Overview

This PR enhances maintainability, thread safety, and logging across the Helm and KubeConfig manager packages.

• Introduces a reusable parseSetValues helper in helm3lib and adds corresponding tests
• Converts shared-state access in kube_config_manager to use sync.RWMutex with withLock/withRLock helpers
• Standardizes logging to use the logger instance and makes Checksums thread-safe

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Replaced sync.Mutex with sync.RWMutex, refactored locking with withLock/withRLock, removed unused method
pkg/kube_config_manager/event_handlers.go	Extracted event-handling logic into helper methods
pkg/kube_config_manager/checksums.go	Added sync.RWMutex, improved thread safety, introduced ensureName
pkg/helm_resources_manager/helm_resources_manager.go	Renamed mutex field, replaced log calls with logger
pkg/helm/post_renderer/post_renderer.go	Nil-check for labels, used maps.Copy for merging
pkg/helm/helm3lib/helm3lib.go	Added parseSetValues and refactored upgradeRelease/Render to use it; updated WithExtraLabels
pkg/helm/helm3lib/helm3lib_test.go	Added tests covering various parseSetValues scenarios

Comments suppressed due to low confidence (3)

pkg/helm/helm3lib/helm3lib.go:464

• The parseSetValues function uses strings.SplitN and strings.TrimSpace but the strings package is not imported. Please add import "strings" to avoid compilation errors.

func parseSetValues(setValues []string) map[string]any {

pkg/kube_config_manager/event_handlers.go:47

• The comment references kcm.m but the mutex field was renamed to mu in the struct. Update the comment to kcm.mu for consistency.

// NOTE: This method must be called with kcm.m locked as it accesses shared state.

pkg/kube_config_manager/event_handlers.go:79

• The comment references kcm.m but the mutex field is now mu. Please update the method documentation to refer to kcm.mu.

// NOTE: This method must be called with kcm.m locked as it accesses shared state.

[Copilot]

Pull Request Overview

This PR enhances code maintainability, thread safety, and logging across the Helm and kube_config_manager packages.

• Introduces a reusable parseSetValues helper to consolidate key-value parsing in helm3lib
• Adds sync.RWMutex and withLock/withRLock helpers for thread-safe operations in kube_config_manager
• Standardizes logging by replacing raw log calls with the structured logger in helm_resources_manager

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Swap Mutex for RWMutex, add locking helpers, refactor handleConfigEvent for shorter locks
pkg/kube_config_manager/event_handlers.go	Extract config-event logic into dedicated handler methods for error, reset, global, module
pkg/kube_config_manager/checksums.go	Make Checksums thread-safe with sync.RWMutex, add helper methods
pkg/helm_resources_manager/helm_resources_manager.go	Rename monitor lock, switch to logger, add proper locking in resource monitor methods
pkg/helm/post_renderer/post_renderer.go	Safely initialize label maps and use maps.Copy for extraLabels
pkg/helm/helm3lib/helm3lib.go	Introduce parseSetValues, update WithExtraLabels to use maps.Copy
pkg/helm/helm3lib/helm3lib_test.go	Add TestParseSetValues unit tests

Comments suppressed due to low confidence (1)

pkg/kube_config_manager/checksums.go:36

• [nitpick] Consider adding unit tests for the Checksums type to cover its thread-safe behavior, especially under concurrent access.

func (c *Checksums) Add(name string, checksum string) {

[Copilot]

Pull Request Overview

This PR improves the maintainability and robustness of the helm and kubeconfig packages by refactoring code for better thread safety, code reusability, and standardized logging while also cleaning up redundant sections.

• Introduces a reusable parseSetValues function to simplify key-value parsing in helm3lib.
• Implements RWMutex-based locking and helper functions (withLock/withRLock) to ensure thread-safe operations in kube config management.
• Standardizes logging by replacing log package direct usage with a logger instance and removes unused code.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Refactored locking mechanism and event handling for thread safety and clarity.
pkg/kube_config_manager/event_handlers.go	Added helper functions for error handling and module events.
pkg/kube_config_manager/checksums.go	Updated Checksums type to be thread-safe with additional helper methods.
pkg/helm_resources_manager/helm_resources_manager.go	Updated mutex naming and logging to use the logger instance consistently.
pkg/helm/post_renderer/post_renderer.go	Enhanced label merging using maps.Copy with proper nil-checks.
pkg/helm/helm3lib/helm3lib_test.go	Added comprehensive tests for the new parseSetValues function.
pkg/helm/helm3lib/helm3lib.go	Replaced duplicated key-value parsing code with a reusable parseSetValues.

[Copilot]

Pull Request Overview

This PR refactors and enhances the Helm and kubeconfig management packages by improving thread safety, adding shared utilities, and standardizing logging.

• Introduces a reusable parseSetValues for DRY parsing of CLI-style key=value slices.
• Adds sync.RWMutex protections and lock helpers (withLock/withRLock) around shared state in the kube_config_manager.
• Replaces ad-hoc log/slog calls with a logger instance and adopts maps.Copy for label merging.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added RWMutex, lock helpers, moved event logic
pkg/kube_config_manager/event_handlers.go	Extracted event handler methods
pkg/kube_config_manager/checksums.go	Made Checksums thread-safe with RWMutex
pkg/helm_resources_manager/helm_resources_manager.go	Renamed mutex fields, standardized logging
pkg/helm/post_renderer/post_renderer.go	Nil-safe label merging using maps.Copy
pkg/helm/helm3lib/helm3lib.go	Added parseSetValues and DRY’d set-value parsing
pkg/helm/helm3lib/helm3lib_test.go	Added unit tests for parseSetValues

Comments suppressed due to low confidence (4)

pkg/kube_config_manager/kube_config_manager.go:7

• The reflect and strconv imports are no longer used after refactoring. Remove these unused imports to clean up the code.

"reflect"

pkg/helm/helm3lib/helm3lib.go:8

• The parseSetValues function uses strings.SplitN and strings.TrimSpace, but the strings package is not imported. Add import "strings" to avoid compilation errors.

"maps"

pkg/kube_config_manager/kube_config_manager.go:300

• Avoid sending on kcm.configEventCh while holding kcm.mu lock to prevent potential deadlocks. Capture the event inside the lock and perform the send after releasing the lock.

kcm.sendEventIfNeeded(eventToSend)

pkg/kube_config_manager/kube_config_manager.go:354

• processBatchDeletedModules reads from shared state but is called outside of kcm.mu lock. Wrap this call in a read or write lock to avoid race conditions.

deletedModulesChanged, deletedModulesStateChanged, deletedModuleMaintenanceChanged := kcm.processBatchDeletedModules(currentModuleNames)

[Copilot]

Pull Request Overview

This PR enhances thread safety, logging consistency, and code reuse across Helm and Kubernetes config manager packages by introducing mutexes, utility functions, and refactoring large methods.

• Introduce parseSetValues for DRY parsing of key/value slices in Helm client
• Replace coarse sync.Mutex with sync.RWMutex and unlock patterns in KubeConfigManager
• Standardize logging calls and break out event‐handling into helper methods
• Add thread-safety to Checksums, helm_resources_manager, and operator_test.go

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Added mu RWMutex, withLock/withRLock helpers, refactored large config‐event methods
pkg/kube_config_manager/event_handlers.go	Extracted event‐handling logic into dedicated methods
pkg/kube_config_manager/checksums.go	Made Checksums thread safe, added ensureName and internal helpers
pkg/helm_resources_manager/helm_resources_manager.go	Renamed mutex, standardized logger usage across monitor methods
pkg/helm/post_renderer/post_renderer.go	Use maps.Copy for merging extra labels
pkg/helm/helm3lib/helm3lib.go	Introduced parseSetValues, reused in upgradeRelease/Render
pkg/helm/helm3lib/helm3lib_test.go	Added comprehensive tests for parseSetValues
pkg/addon-operator/operator_test.go	Switched to thread-safe TaskHandleHistory

Comments suppressed due to low confidence (2)

pkg/kube_config_manager/kube_config_manager.go:283

• [nitpick] handleBatchConfigEvent is quite long and handles multiple concerns; consider splitting it into smaller functions (e.g., separate methods for processing global changes, module updates, and deletions) to improve readability and simplify future maintenance.

func (kcm *KubeConfigManager) handleBatchConfigEvent(obj config.Event) {

pkg/kube_config_manager/event_handlers.go:137

• processBatchDeletedModules contains non-trivial logic for module deletion; adding unit tests for this method will help ensure deletion events, state changes, and maintenance flags are handled correctly.

func (kcm *KubeConfigManager) processBatchDeletedModules(

[Copilot]

Pull Request Overview

This PR enhances thread safety across kubeconfig and helm packages, introduces a robust parseSetValues utility with accompanying tests, standardizes logging, and simplifies map operations using Go’s maps package.

• Converts various mutexes to sync.RWMutex and adds withLock/withRLock helpers for safer concurrency.
• Implements parseSetValues for Helm set‐value parsing and adds unit tests covering edge cases.
• Refactors logging calls to use structured logger instances and replaces manual map copies with maps.Copy.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/kube_config_manager/kube_config_manager.go	Switched to sync.RWMutex, added lock helpers, refactored event handling to minimize lock time
pkg/kube_config_manager/event_handlers.go	Extracted error/reset/global/module handlers for clarity and locked access
pkg/kube_config_manager/checksums.go	Made Checksums thread-safe with sync.RWMutex and helper methods
pkg/helm_resources_manager/helm_resources_manager.go	Renamed mutex field to mu, standardized logging and locking around monitor map
pkg/helm/post_renderer/post_renderer.go	Guarded against nil labels and used maps.Copy for extraLabels
pkg/helm/helm3lib/helm3lib.go	Added parseSetValues, replaced inline parsing logic with helper, used maps.Copy
pkg/helm/helm3lib/helm3lib_test.go	Added comprehensive unit tests for parseSetValues
pkg/addon-operator/operator_test.go	Switched tests to use thread-safe TaskHandleHistory

Comments suppressed due to low confidence (3)

pkg/kube_config_manager/checksums.go:3

• [nitpick] Rename the mutex field to mu to match Go conventions and maintain consistency with other types (e.g., using mu in KubeConfigManager).

type Checksums struct {

pkg/kube_config_manager/checksums.go:1

• [nitpick] There are currently no unit tests exercising concurrent access to Checksums. Consider adding tests that concurrently call Add, Remove, Copy, and HasEqualChecksum to verify thread safety under load.

package kube_config_manager

pkg/helm/helm3lib/helm3lib.go:466

• The implementation of parseSetValues uses strings.SplitN and strings.TrimSpace but the strings package is not imported. Add import "strings" at the top of this file to prevent compilation errors.

func parseSetValues(setValues []string) map[string]any {