broaden default Quay OAuth scopes by helsus · Pull Request #310 · rh-ecosystem-edge/enclave

helsus · 2026-04-28T11:16:02Z

Description

Adds user:admin and org:admin to the default scopes. oauth_setup.yaml skips org/app creation when quay-oauth-credentials exists and regenerates the token only when scopes drift. quay-oauth-sync tag in 06-day2.yaml covers existing clusters.

Open question

Is 06-day2.yaml the right place to add sync for upgrading existing clusters or should it live in a standalone playbook?

Testing:

Fresh install -- new scopes
Existing cluster with old scopes -- sync regenerates
Idempotency -- re-running is a no-op
Order-insensitive scope match -- no regen
Legacy Secret missing the scopes key -- sync handles it
Token actually carries the new scopes/permissions

Summary by CodeRabbit

New Features
- Expanded Quay OAuth application scopes to include user administrator and organization administrator permissions.
Documentation
- Introduced new runbook documenting the process for updating Quay OAuth access tokens with verification and validation steps.

Adds user:admin and org:admin to the default scopes. oauth_setup.yaml now skips org/app creation when quay-oauth-credentials already exists and only regenerates the token when scopes drift. A quay-oauth-sync tag in 06-day2.yaml covers existing clusters.

coderabbitai · 2026-04-28T11:16:10Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d6483462-cc09-42a4-970b-3937e0396e7a

📥 Commits

Reviewing files that changed from the base of the PR and between 764f895 and 2f0294d.

📒 Files selected for processing (1)

docs/QUAY_OAUTH_TOKEN_REGEN.md

✅ Files skipped from review due to trivial changes (1)

docs/QUAY_OAUTH_TOKEN_REGEN.md

Walkthrough

OAuth scopes for Quay are expanded to include user and organization admin permissions. A new documentation file provides a runbook for regenerating Quay OAuth access tokens after scope changes, including prerequisites, script steps, and verification procedures.

Changes

Cohort / File(s)	Summary
OAuth Configuration `defaults/quay_operator.yaml`	Updated `quayOAuthApp.scopes` to add `user:admin` and `org:admin` permissions alongside existing repository and user read scopes.
Documentation `docs/QUAY_OAUTH_TOKEN_REGEN.md`	New runbook documenting the process for regenerating Quay OAuth tokens after scope modifications, including prerequisite tools, token request script, Secret patching steps, and verification commands.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'broaden default Quay OAuth scopes' directly and clearly describes the main change: expanding the default Quay OAuth scopes to include user:admin and org:admin permissions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

playbooks/06-day2.yaml (1)
86-94: Consider making OAuth sync explicitly opt-in for Day-2 runs.

Lines 86-94 add a mutating credential/scope reconciliation step to the default Day-2 flow. Given the blast radius, a standalone playbook (or an extra opt-in var) would make operational intent clearer while keeping --tags quay-oauth-sync for targeted runs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@playbooks/06-day2.yaml` around lines 86 - 94, The new "Sync Quay OAuth
credentials with desired scopes" task currently runs by default when
quayOAuthApp.enabled is true; change it to be explicit opt‑in by adding an
additional condition or variable (e.g., quayOAuthSyncEnabled default false) and
update the task's when to require both quayOAuthApp.enabled and
quayOAuthSyncEnabled, or extract the include_tasks
(../operators/quay-operator/oauth_setup.yaml) into a separate standalone
playbook that operators run intentionally; preserve the existing tag
quay-oauth-sync so targeted runs still work.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@operators/quay-operator/oauth_setup.yaml`:
- Around line 27-33: The current set_fact uses direct indexing into
r_quay_oauth_secret_existing.resources[0].data which will error if the Secret is
present but missing keys; change those to safe lookups (e.g., use the dict .get
method or |default fallbacks) when assigning quay_oauth_client_id,
quay_oauth_client_secret, and quay_oauth_current_scopes so missing keys yield
empty strings rather than raising (for example:
r_quay_oauth_secret_existing.resources[0].data.get('client-id','') | b64decode
or r_quay_oauth_secret_existing.resources[0].data.get('scopes','') | default('')
| b64decode), keeping the quay_oauth_secret_exists guard unchanged.

---

Nitpick comments:
In `@playbooks/06-day2.yaml`:
- Around line 86-94: The new "Sync Quay OAuth credentials with desired scopes"
task currently runs by default when quayOAuthApp.enabled is true; change it to
be explicit opt‑in by adding an additional condition or variable (e.g.,
quayOAuthSyncEnabled default false) and update the task's when to require both
quayOAuthApp.enabled and quayOAuthSyncEnabled, or extract the include_tasks
(../operators/quay-operator/oauth_setup.yaml) into a separate standalone
playbook that operators run intentionally; preserve the existing tag
quay-oauth-sync so targeted runs still work.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c1ee4e63-ba29-4d9d-9503-5a7e4f72aec8

📥 Commits

Reviewing files that changed from the base of the PR and between 992b189 and 764f895.

📒 Files selected for processing (3)

defaults/quay_operator.yaml
operators/quay-operator/oauth_setup.yaml
playbooks/06-day2.yaml

coderabbitai · 2026-04-28T11:32:07Z

+- name: Reuse credentials from existing Secret
+  no_log: true
+  when: quay_oauth_secret_exists
+  ansible.builtin.set_fact:
+    quay_oauth_client_id: "{{ r_quay_oauth_secret_existing.resources[0].data['client-id'] | b64decode }}"
+    quay_oauth_client_secret: "{{ r_quay_oauth_secret_existing.resources[0].data['client-secret'] | b64decode }}"
+    quay_oauth_current_scopes: "{{ r_quay_oauth_secret_existing.resources[0].data['scopes'] | default('') | b64decode }}"


⚠️ Potential issue | 🟠 Major

Handle incomplete existing OAuth Secrets defensively.

At Line 31 and Line 32, direct key indexing (data['client-id'], data['client-secret']) will fail hard if the Secret exists but is incomplete. This blocks recovery paths during upgrades/drift scenarios.

Suggested hardening

- name: Reuse credentials from existing Secret no_log: true when: quay_oauth_secret_exists ansible.builtin.set_fact: - quay_oauth_client_id: "{{ r_quay_oauth_secret_existing.resources[0].data['client-id'] | b64decode }}" - quay_oauth_client_secret: "{{ r_quay_oauth_secret_existing.resources[0].data['client-secret'] | b64decode }}" - quay_oauth_current_scopes: "{{ r_quay_oauth_secret_existing.resources[0].data['scopes'] | default('') | b64decode }}" + quay_oauth_client_id: "{{ (r_quay_oauth_secret_existing.resources[0].data | default({})).get('client-id', '') | b64decode }}" + quay_oauth_client_secret: "{{ (r_quay_oauth_secret_existing.resources[0].data | default({})).get('client-secret', '') | b64decode }}" + quay_oauth_current_scopes: "{{ (r_quay_oauth_secret_existing.resources[0].data | default({})).get('scopes', '') | b64decode }}"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@operators/quay-operator/oauth_setup.yaml` around lines 27 - 33, The current set_fact uses direct indexing into r_quay_oauth_secret_existing.resources[0].data which will error if the Secret is present but missing keys; change those to safe lookups (e.g., use the dict .get method or |default fallbacks) when assigning quay_oauth_client_id, quay_oauth_client_secret, and quay_oauth_current_scopes so missing keys yield empty strings rather than raising (for example: r_quay_oauth_secret_existing.resources[0].data.get('client-id','') | b64decode or r_quay_oauth_secret_existing.resources[0].data.get('scopes','') | default('') | b64decode), keeping the quay_oauth_secret_exists guard unchanged.

pgodowski · 2026-04-28T11:32:26Z

@oourfali @maorfr can you please take a look?
thanks

maorfr · 2026-04-28T12:13:08Z

/ok-to-test

maorfr

this PR introduces a configuration change together with somewhat of a migration to put it in place.

not sure this is a desired pattern at this point.

i suggest to submit the configuration change only (together with a document perhaps). generally speaking, i think quay-oauth would be a nice plugin (as in, extract this entire functionality to a plugin).

maorfr · 2026-04-28T12:36:15Z

  when:
    - quay_oauth_client_id is defined
    - quay_oauth_client_id | length > 0
+    - not quay_oauth_secret_exists or (quay_oauth_current_scopes.split() | sort) != (quayOAuthApp.scopes.split() | sort)


what happens to current token if scopes have changed?

It will remain in Quay. I haven't found a reliable way to revoke it via automation, so I'll document how to revoke it manually via UI.

chunlongliang-ibm · 2026-04-28T14:22:00Z

is the oauth token only available to MSP admin users, and NOT accessible to tenant?

helsus · 2026-04-29T07:39:21Z

is the oauth token only available to MSP admin users, and NOT accessible to tenant?

Yes, it is only available to MSP admins.

helsus · 2026-04-29T07:42:38Z

this PR introduces a configuration change together with somewhat of a migration to put it in place.

not sure this is a desired pattern at this point.

i suggest to submit the configuration change only (together with a document perhaps). generally speaking, i think quay-oauth would be a nice plugin (as in, extract this entire functionality to a plugin).

Good idea. I've updated the PR to include only the config and doc on how to reissue the token on live clusters.

github-actions Bot added deployment Deployment-related changes operators Operator installation/config labels Apr 28, 2026

helsus marked this pull request as ready for review April 28, 2026 11:25

coderabbitai Bot reviewed Apr 28, 2026

View reviewed changes

maorfr requested changes Apr 28, 2026

View reviewed changes

helsus added 2 commits April 29, 2026 09:32

Revert playbook changes

fba5f56

Add documentation on how to reissue a Quay token for existing clusters

2f0294d

github-actions Bot removed the operators Operator installation/config label Apr 29, 2026

helsus requested a review from maorfr April 29, 2026 07:41

maorfr approved these changes Apr 29, 2026

View reviewed changes

maorfr merged commit 7ccadb8 into rh-ecosystem-edge:main Apr 29, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

broaden default Quay OAuth scopes#310

broaden default Quay OAuth scopes#310
maorfr merged 3 commits intorh-ecosystem-edge:mainfrom
helsus:ib-add-scopes-to-default-user

helsus commented Apr 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Apr 28, 2026

Uh oh!

pgodowski commented Apr 28, 2026

Uh oh!

maorfr commented Apr 28, 2026

Uh oh!

maorfr left a comment •

edited

Loading

Uh oh!

maorfr Apr 28, 2026

Uh oh!

helsus Apr 29, 2026

Uh oh!

chunlongliang-ibm commented Apr 28, 2026

Uh oh!

helsus commented Apr 29, 2026

Uh oh!

helsus commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

helsus commented Apr 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Open question

Testing:

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

pgodowski commented Apr 28, 2026

Uh oh!

maorfr commented Apr 28, 2026

Uh oh!

maorfr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maorfr Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

helsus Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

chunlongliang-ibm commented Apr 28, 2026

Uh oh!

helsus commented Apr 29, 2026

Uh oh!

helsus commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

helsus commented Apr 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading

maorfr left a comment •

edited

Loading