Skip to content

fix(helm): support user defined namespace with nuvlaNamespaceOverride value#450

Merged
schaubl merged 6 commits intomainfrom
helm-custom-ns
Jul 15, 2025
Merged

fix(helm): support user defined namespace with nuvlaNamespaceOverride value#450
schaubl merged 6 commits intomainfrom
helm-custom-ns

Conversation

@konstan
Copy link
Contributor

@konstan konstan commented Jul 14, 2025

@konstan konstan requested a review from Copilot July 14, 2025 10:06

This comment was marked as outdated.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 14, 2025

Test Results

67 tests  ±0   67 ✅ ±0   1s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit c897725. ± Comparison against base commit c85badb.

♻️ This comment has been updated with latest results.

@konstan konstan requested a review from Copilot July 14, 2025 14:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for user-defined namespaces in Helm deployments by allowing a nuvlaNamespaceOverride key in chart values to replace the default namespace and ensuring the override flows through action parameters and connector calls.

  • Extend _get_action_params_helm to include full module content for Helm actions.
  • Introduce _override_namespace in the Kubernetes connector to apply a namespace override from chart values.
  • Propagate the overridden namespace through Helm operations and update get_objects in the k8s driver to accept an explicit namespace.

Reviewed Changes

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

Show a summary per file
File Description
nuvla/job_engine/job/actions/deployment_stop.py Include module_content in Helm-specific action parameters
nuvla/job_engine/job/actions/deployment_state.py Pass deployment.data into get_services when using Helm connector
nuvla/job_engine/connector/kubernetes.py Add yaml import, implement NAMESPACE_OVERRIDE_KEY and override logic
nuvla/job_engine/connector/k8s_driver.py Update get_objects signature to accept namespace and fallback
nuvla/job_engine/connector/helm_driver.py Remove unused from_base64 import
Comments suppressed due to low confidence (5)

nuvla/job_engine/connector/k8s_driver.py:722

  • The docstring for get_objects should be updated to include the new namespace parameter and explain its behavior, ensuring API consumers are aware of the change.
    def get_objects(self, deployment_uuid, object_kinds: List[str], namespace=None) -> List[dict]:

nuvla/job_engine/connector/kubernetes.py:162

  • [nitpick] Consider adding unit tests for the _override_namespace method, covering cases where the YAML contains a namespace override, invalid YAML, and no override, to ensure correct behavior.
    def _override_namespace(self, namespace: str, values_yaml: str) -> str:

nuvla/job_engine/job/actions/deployment_stop.py:49

  • [nitpick] Add tests for _get_action_params_helm to verify that module_content is correctly included in the returned params for Helm deployments.
    def _get_action_params_helm(self, deployment: dict) -> dict:

nuvla/job_engine/job/actions/deployment_state.py:35

  • [nitpick] Include tests for get_application_state when connector_name is CONNECTOR_KIND_HELM, ensuring kwargs['deployment'] is correctly passed to get_services.
        if connector_name == CONNECTOR_KIND_HELM:

nuvla/job_engine/job/actions/deployment_state.py:35

  • The code uses kwargs['deployment'] but kwargs is not defined in this scope. Please ensure kwargs is initialized or passed into the method before usage.
        if connector_name == CONNECTOR_KIND_HELM:

@konstan konstan requested a review from schaubl July 14, 2025 14:51
@schaubl schaubl changed the title feat: support user defined namespace in Helm fix(helm): support user defined namespace with nuvlaNamespaceOverride value Jul 15, 2025
@schaubl schaubl merged commit f2fd6df into main Jul 15, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants