Support for optional field in configmap / secret ref#181
Support for optional field in configmap / secret ref#181GuyCarmy wants to merge 2 commits intorundeck-plugins:masterfrom
Conversation
|
Hi @ltamaster, are you still maintaining this repo? |
|
Hi @fdevans, I see you were active in this repo last week, any chance you can take a look at this small PR? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the optional field in configmap and secret references in Kubernetes job creation. The changes enable users to specify whether a configmap or secret reference should be treated as optional during job execution.
- Extracts configmap and secret references to separate variables for cleaner code
- Adds support for the
optionalfield in both V1ConfigMapEnvSource and V1SecretEnvSource - Fixes parameter passing by using named parameters instead of positional ones
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| config_map_ref=client.V1ConfigMapEnvSource( | ||
| env_from_data['configMapRef']['name'] | ||
| name=config_map_ref['name'], | ||
| optional=config_map_ref.get('optional') | ||
| ) |
There was a problem hiding this comment.
The original code was passing the name as a positional argument, but V1ConfigMapEnvSource expects named parameters. However, the API typically expects the optional parameter to be a boolean. Using .get('optional') could return None, which may not be handled correctly by the Kubernetes client. Consider using .get('optional', False) to provide a default boolean value.
| secret_ref=client.V1SecretEnvSource( | ||
| env_from_data['secretRef']['name'] | ||
| name=secret_ref['name'], | ||
| optional=secret_ref.get('optional') | ||
| ) |
There was a problem hiding this comment.
Similar to the configmap reference, the optional parameter should have a boolean default value. Using .get('optional') could return None, which may cause issues with the Kubernetes client. Consider using .get('optional', False) to ensure a proper boolean value is always passed.
|
@fdevans fixed according to the copilot suggestions |
|
@GuyCarmy Thanks for the contribution! This is a useful feature addition. Code ReviewThe implementation looks good:
Requests Before Merge1. Rebase on Latest MasterYour branch is behind master by several commits, including important security updates (urllib3 and kubernetes client upgrades for CVE fixes). Please rebase on the latest master: git fetch upstream
git rebase upstream/master
# Resolve any conflicts if needed
git push --force-with-lease2. Add DocumentationPlease add an example to the README showing how to use the new In the "Create / Delete / Re-run Jobs" section, add an example showing:
This will help users understand when and how to use this feature. Example Documentation FormatenvFrom:
- configMapRef:
name: my-config
optional: true # Job will start even if my-config doesn't exist
- secretRef:
name: my-secret
optional: false # Job will fail if my-secret doesn't exist (default)Once you've rebased and added the documentation, we'll be ready to merge. Thanks again for the contribution! |
No description provided.