Repository navigation
[ACTP] Add Private Action Runner feature configuration - #3653
martin-prevost1 wants to merge 3 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 9e4a31b | Docs | View more details | Give us feedback! |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
| // Defaults to true for Agent images supporting split mode (7.84.0+). | ||
| // Older Agent images always use the legacy mode. | ||
| // +optional | ||
| SplitEnabled *bool `json:"splitEnabled,omitempty"` |
There was a problem hiding this comment.
this probably shouldn't be exposed as a top level config and can be kept as an annotation
| // Takes precedence over actions_allowlist in ConfigData. | ||
| // +optional | ||
| // +listType=atomic | ||
| ActionsAllowlist *[]string `json:"actionsAllowlist,omitempty"` |
There was a problem hiding this comment.
maybe we can keep it in configdata ? especially since we are heavily discussing what we are doing about it, we might not want to introduce it in this PR
There was a problem hiding this comment.
Do you have an idea of the timeline for removing the allowlist? If it is planned for several quarters from now, it may be worth keeping it. But if it is happening very soon, I agree with you. 🙂
There was a problem hiding this comment.
The discussions are happening ~now but there are many open questions on it. I think showing an example with the allowlist in the configdata is a good solution for now
There was a problem hiding this comment.
FYI this might conflict with #3618, you might want to sync on it
| // K8sRemediationEnabled grants cluster-wide permissions for Kubernetes remediation actions. | ||
| // Default: false | ||
| // +optional | ||
| K8sRemediationEnabled *bool `json:"k8sRemediationEnabled,omitempty"` |
There was a problem hiding this comment.
I think this might conflict / double usage with
spec:
features:
kubernetesActions:
enabled: true
which already exists
Keep allowlists in configData and retain advanced split and remediation controls through existing annotations and Kubernetes Actions. Update configuration resolution, examples, tests, and generated schemas.
| : Overrides the shared enabled setting for the Cluster Agent. | ||
|
|
||
| `features.privateActionRunner.configData` | ||
| : ConfigData contains YAML configuration, including the private_action_runner key, shared by both agents. The enabled field is controlled by Enabled instead. Configure allowed action patterns with private_action_runner.actions_allowlist. Default: "private_action_runner:\n enabled: true\n" |
There was a problem hiding this comment.
instead of what?
| : ConfigData contains YAML configuration, including the private_action_runner key, shared by both agents. The enabled field is controlled by Enabled instead. Configure allowed action patterns with private_action_runner.actions_allowlist. Default: "private_action_runner:\n enabled: true\n" | |
| : ConfigData contains YAML configuration, including the private_action_runner key, shared by both agents. The enabled field is controlled by Enabled. Configure allowed action patterns with private_action_runner.actions_allowlist. Default: "private_action_runner:\n enabled: true\n" |
What does this PR do?
Adds
spec.features.privateActionRunnerwith sharedenabledandconfigDatasettings for the Node Agent and Cluster Agent, plus per-agent overrides. ComponentconfigDatareplaces the shared YAML, including its allowlist.Existing annotation configuration remains supported. Split mode stays controlled by annotation; Kubernetes actions use the existing
features.kubernetesActionsfeature.Test
make lint, API/CRD generation, and server-side validation of all six examples passed.