execute clairctl export on landing zone itself and not within container#30
execute clairctl export on landing zone itself and not within container#30agonzalezrh merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds clairctl support: schema and default metadata, an Ansible task to download the clairctl binary, and updates the Clair disconnected export playbook to invoke clairctl directly instead of querying Pods or using a container export step. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
operators/quay-operator/clair_disconnected.yaml (1)
25-26: Useansible.builtin.command+argvinstead ofshellfor this direct exec.Line 25-26 invokes a binary directly with no shell operators or special expansions—only direct arguments and Jinja2 variables. The
commandmodule withargvhandles Jinja2 variables naturally and passes each argument distinctly, avoiding quoting/escaping edge cases thatshellintroduces.Suggested refactor
- name: Export vulnerability data on Landing Zone ansible.builtin.command: argv: - "{{ workingDir }}/bin/clairctl" - "--config" - "{{ workingDir }}/data/clair/config.yaml" - "export-updaters" - "{{ workingDir }}/data/clair/updates.json.gz"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operators/quay-operator/clair_disconnected.yaml` around lines 25 - 26, Replace the ansible.builtin.shell task that runs the clairctl binary with ansible.builtin.command using the argv form so arguments are passed safely; specifically swap the module name to ansible.builtin.command and supply argv as a list with entries for the executable ("{{ workingDir }}/bin/clairctl"), the --config flag and its value ("--config", "{{ workingDir }}/data/clair/config.yaml"), the subcommand ("export-updaters"), and the output path ("{{ workingDir }}/data/clair/updates.json.gz") so no shell interpretation or quoting issues occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@defaults/control_binaries.yaml`:
- Around line 15-17: The schema validation fails because control_binaries.yaml
now contains a clairctl entry but schemas/control_binaries.yaml does not allow
it; update schemas/control_binaries.yaml to add a "clairctl" property under
control_binaries (or the same object where other binaries are defined) using the
identical shape used by existing binaries (url and checksum string properties,
required as appropriate) so the new key validates while preserving
additionalProperties: false and existing constraints.
---
Nitpick comments:
In `@operators/quay-operator/clair_disconnected.yaml`:
- Around line 25-26: Replace the ansible.builtin.shell task that runs the
clairctl binary with ansible.builtin.command using the argv form so arguments
are passed safely; specifically swap the module name to ansible.builtin.command
and supply argv as a list with entries for the executable ("{{ workingDir
}}/bin/clairctl"), the --config flag and its value ("--config", "{{ workingDir
}}/data/clair/config.yaml"), the subcommand ("export-updaters"), and the output
path ("{{ workingDir }}/data/clair/updates.json.gz") so no shell interpretation
or quoting issues occur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebfbc3a2-be3a-4979-9418-db89b3d7a51a
📒 Files selected for processing (3)
defaults/control_binaries.yamloperators/quay-operator/clair_disconnected.yamlplaybooks/tasks/download_control_binaries.yaml
running the command clairctl in the landing zone directly to avoid spinning up a container using podman.
this makes for a more consistent approach of obtaining control binaries and using them.
related to https://github.com/gori-project/GoRI/issues/818
this PR gets the same clairctl version as used in quay operator version 3.15: https://github.com/quay/clair/releases/tag/v4.8.0
Summary by CodeRabbit
New Features
Improvements