fix(harbor): the robot reconcile has to read under --check - #18
Open
kholisrag wants to merge 1 commit into
Open
Conversation
Found by running it. `ansible.builtin.uri` does not support check mode, so both reads SKIPPED under --check, the map task saw no data, and the "robot exists" assert failed against a Harbor that holds the robot perfectly well. A dry run that reports a problem it invented is worse than no dry run — it is the one mode whose whole job is to be trusted before a write. Both reads are GETs carrying changed_when: false, so `check_mode: false` on them changes nothing and leaves the PUT as the only simulated step, which is what --check is for here. Also declares the first robot in the vault, and corrects the report line, which said "widened this run" while measuring the delta taken BEFORE the PUT — so a --check run claimed a change it had not made. Verified against the live Harbor: --check reports one grant to add, the real run applies it (changed=1), and a second run reports 0 with changed=0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #17, found by actually running it.
The bug
ansible.builtin.uridoes not support check mode. Under--checkboth readsskipped, the map task saw no data, and Assert every declared robot exists
failed against a Harbor that holds the robot perfectly well:
A dry run that reports a problem it invented is worse than no dry run —
--checkis the one mode whose entire job is to be trusted before a write, and this is a
file whose entire job is to refuse a dangerous write.
Both reads are
GETs carryingchanged_when: false, socheck_mode: falsechanges nothing about them and leaves the
PUTas the only simulated step.Also
vars/vault.yaml. Reading its live grants wasworth doing: the ticket that prompted this said the robot held four, and it
holds fourteen —
immutable-tag:*andtag-retention:*onproject:*,which it needs to write per-project policy. A declaration built from the
ticket would have tried to revoke ten of them, and the widen-only guard would
have stopped the run rather than doing it. That is the guard earning its keep
on its first real use.
the delta taken before the PUT — so a
--checkrun claimed a change ithad not made. It now says "with grants to add", which is true in both modes.
Verified against the live Harbor
And downstream, in the repository this grant exists for: its playbook now gets
past the 403 that prompted all of this, 22 tasks passing where it used to stop
at 20.
Unrelated, but you should know
--tags robotscannot reach the robot tasks without--skip-tags always,because preflight fails:
That assertion compares
ansible_facts['memtotal_mb']against3800, andmemtotal_mbis always a few hundred MiB below the RAM the VM was given — thekernel and firmware reserve it. So a host sitting exactly on its 4 GiB balloon
floor reports ~3789 and fails a check meant to catch a VM with less than 4 GiB.
Not touched here: changing a safety assertion is its own decision, and this PR
should not carry it.