HYPERFLEET-482 - fix: remove references to old WIF #31
Conversation
WalkthroughThis pull request updates Helm chart documentation and configuration examples. It adds documentation in the README describing how adapter.yaml and broker.yaml ConfigMap keys and mount paths are created when config.adapterYaml and broker.yaml settings are provided. Additionally, it removes outdated GCP Workload Identity Federation annotation examples and simplifies the ServiceAccount configuration documentation by replacing the annotations example with a name field example. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/README.md (1)
161-169: “With GCP Workload Identity” example no longer shows WI-specific config.Line 161 still labels this as a WI example, but the snippet only sets broker fields. This now reads identical to the basic install example and may mislead users about required WI configuration. Consider either removing the section or replacing it with the current, correct WI-specific settings (e.g., serviceAccount annotations or serviceAccount name strategy, depending on your chart).
🤖 Fix all issues with AI agents
In `@charts/README.md`:
- Around line 55-59: Update the README bullets to state the gating conditions:
clarify that the adapter.yaml ConfigMap key, mount at /etc/adapter/adapter.yaml,
and ADAPTER_CONFIG_PATH are created only when both config.enabled: true and
config.adapterYaml are set (reference config.enabled and config.adapterYaml);
likewise clarify the broker.yaml ConfigMap key and mount exist only when both
broker.create: true and broker.yaml are set (reference broker.create and
broker.yaml). Make the wording for adapter.yaml and broker.yaml symmetrical and
unambiguous so readers know both flags/values must be present for the ConfigMap
keys/mounts/env vars to be created.
| When `config.adapterYaml` is set: | ||
|
|
||
| - Creates `adapter.yaml` key in ConfigMap | ||
| - Mounts at `/etc/adapter/adapter.yaml` | ||
| - Sets `ADAPTER_CONFIG_PATH=/etc/adapter/adapter.yaml` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Explore chart structure
find charts -type f -name "*.yaml" -o -name "*.yml" -o -name "*.tpl" | head -20Repository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 350
🏁 Script executed:
# Search for the relevant template logic
rg -l "adapter\.yaml|ADAPTER_CONFIG_PATH" charts --type yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 166
🏁 Script executed:
# Look at the main deployment/pod template
find charts -name "deployment.yaml" -o -name "statefulset.yaml" -o -name "_*.tpl" | head -10Repository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 145
🏁 Script executed:
# Check configmap.yaml for adapter.yaml handling
cat -n charts/templates/configmap.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 1018
🏁 Script executed:
# Check broker-configmap.yaml for broker.yaml handling
cat -n charts/templates/broker-configmap.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 1333
🏁 Script executed:
# Check deployment.yaml for env var wiring and conditions
cat -n charts/templates/deployment.yaml | head -100Repository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 4447
🏁 Script executed:
# Get the rest of deployment.yaml focusing on volumeMounts and volumes sections
cat -n charts/templates/deployment.yaml | tail -100Repository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 4672
🏁 Script executed:
# Search for ADAPTER_CONFIG_PATH and BROKER_CONFIG_FILE in deployment template
rg "ADAPTER_CONFIG_PATH|BROKER_CONFIG_FILE" charts/templates/deployment.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 161
🏁 Script executed:
# Get context around those env var names in deployment.yaml
rg -B2 -A2 "ADAPTER_CONFIG_PATH|BROKER_CONFIG_FILE" charts/templates/deployment.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 453
Add required gating conditions to clarify ConfigMap/ENV behavior.
The bullets at lines 55-59 and 75-79 omit that these behaviors require additional conditions. The adapter.yaml ConfigMap key and mount only exist when both config.enabled: true and config.adapterYaml is set. Similarly, broker.yaml ConfigMap key and mount only exist when both broker.create: true and broker.yaml is set.
Update the text to clarify:
Suggested fix
-When `config.adapterYaml` is set:
+When `config.enabled: true` and `config.adapterYaml` is set:
-When `broker.yaml` is set:
+When `broker.create: true` and `broker.yaml` is set:🤖 Prompt for AI Agents
In `@charts/README.md` around lines 55 - 59, Update the README bullets to state
the gating conditions: clarify that the adapter.yaml ConfigMap key, mount at
/etc/adapter/adapter.yaml, and ADAPTER_CONFIG_PATH are created only when both
config.enabled: true and config.adapterYaml are set (reference config.enabled
and config.adapterYaml); likewise clarify the broker.yaml ConfigMap key and
mount exist only when both broker.create: true and broker.yaml are set
(reference broker.create and broker.yaml). Make the wording for adapter.yaml and
broker.yaml symmetrical and unambiguous so readers know both flags/values must
be present for the ConfigMap keys/mounts/env vars to be created.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xueli181114 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 69ea37f
into
openshift-hyperfleet:main
2 participants
https://issues.redhat.com/browse/HYPERFLEET-482
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.