diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1bb81e8c..78782ab0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -15,6 +15,15 @@ - [ ] Development environment change - [ ] Configuration change +## Definition of Done + + +- [ ] This PR does NOT change an API or collection behavior (skip the lines below) +- [ ] `ansible.platform` module/plugin code updated for the API change +- [ ] `DOCUMENTATION` string updated (params, RETURN values, EXAMPLES) +- [ ] Integration test added/updated and passing in CI +- [ ] CasC notification completed (see section below) + ## Self-Review Checklist - [ ] I have performed a self-review of my code @@ -26,6 +35,7 @@ - [ ] I have tested the changes in my local environment - [ ] Existing playbook FQCNs are preserved (no renames without a redirect in `meta/routing.yml`) - [ ] Deprecated parameters include a `deprecated:` block in `DOCUMENTATION` with removal version +- [ ] Return values match the shared structure from `plugin_utils` (no one-off return formats) ## Testing Instructions diff --git a/.github/workflows/casc-notify-check.yml b/.github/workflows/casc-notify-check.yml index c18039f9..5a846ddb 100644 --- a/.github/workflows/casc-notify-check.yml +++ b/.github/workflows/casc-notify-check.yml @@ -7,11 +7,13 @@ on: - opened - synchronize - reopened + - edited # re-run when PR description is updated (e.g. author ticks boxes) paths: - 'plugins/modules/**' - 'plugins/action/**' - 'plugins/plugin_utils/**' - 'plugins/doc_fragments/**' + - 'plugins/lookup/**' permissions: pull-requests: write @@ -19,96 +21,212 @@ permissions: jobs: casc-notify-check: - name: Check CasC notification requirement + name: CasC notification check runs-on: ubuntu-latest steps: - # Required for fork PRs under pull_request_target (reads PR files only; no secrets execution path). - - uses: actions/checkout@v3 + # Safe read-only checkout of the PR head for diffing only. + # No secrets are executed on the PR code — this step only reads file paths and diff. + - uses: actions/checkout@v4 with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - allow-unsafe-pr-checkout: true - - name: Get list of changed files - id: changed-files + - name: Fetch base branch and compute diff + id: changes run: | git fetch origin ${{ github.base_ref }} - CHANGED=$(git diff --name-only origin/${{ github.base_ref }}...HEAD) + BASE=origin/${{ github.base_ref }} + + CHANGED=$(git diff --name-only ${BASE}...HEAD) echo "changed_files<> $GITHUB_OUTPUT echo "$CHANGED" >> $GITHUB_OUTPUT echo "EOF" >> $GITHUB_OUTPUT + # Full diff of plugin paths for content-level detection + git diff ${BASE}...HEAD -- plugins/ > /tmp/plugin_diff.txt || true + - name: Detect CasC trigger areas - id: detect-triggers + id: detect run: | - CHANGED="${{ steps.changed-files.outputs.changed_files }}" - TRIGGERS="" - - # New or changed modules - if echo "$CHANGED" | grep -qE '^plugins/modules/'; then - NEW_MODULES=$(echo "$CHANGED" | grep -E '^plugins/modules/' | grep -v '__pycache__') - if [ -n "$NEW_MODULES" ]; then - TRIGGERS="${TRIGGERS}\n- **Module changes**: \`$(echo "$NEW_MODULES" | tr '\n' ' ')\`" + CHANGED="${{ steps.changes.outputs.changed_files }}" + BASE=origin/${{ github.base_ref }} + INFORMATIONAL="" + BREAKING="" + + # ── DAB RBAC / cross-service modules ───────────────────────────── + # These modules affect RBAC inside Controller, EDA, and Hub. + # Even small changes can silently break cross-service permissions. + DAB_RBAC_MODULES="organization team user role_definition role_team_assignment role_user_assignment" + DAB_CHANGED="" + for mod in $DAB_RBAC_MODULES; do + if echo "$CHANGED" | grep -qE "(plugins/modules/${mod}\.py|plugins/action/${mod}\.py|plugins/plugin_utils/api/v1/${mod}\.py)"; then + DAB_CHANGED="${DAB_CHANGED} ${mod}" fi + done + if [ -n "$DAB_CHANGED" ]; then + BREAKING="${BREAKING}\n- **Cross-service DAB RBAC modules changed**:$(echo "$DAB_CHANGED" | tr ' ' '\n' | grep -v '^$' | sed 's/^/ \`/' | sed 's/$/ \`/' | tr '\n' ' ') — affects RBAC inside Controller, EDA, and Hub. ALL PDTs must review." fi - # Action plugin changes - if echo "$CHANGED" | grep -qE '^plugins/action/'; then - ACTION_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/action/' | grep -v '__pycache__') - if [ -n "$ACTION_CHANGES" ]; then - TRIGGERS="${TRIGGERS}\n- **Action plugin changes**: \`$(echo "$ACTION_CHANGES" | tr '\n' ' ')\`" - fi + # ── New modules or action plugins ───────────────────────────────── + NEW_FILES=$(git diff --name-only --diff-filter=A ${BASE}...HEAD \ + -- plugins/modules/ plugins/action/ 2>/dev/null \ + | grep -v '__pycache__' || true) + if [ -n "$NEW_FILES" ]; then + FILES=$(echo "$NEW_FILES" | tr '\n' ' ') + INFORMATIONAL="${INFORMATIONAL}\n- **New module/action plugin**: \`${FILES}\`" fi - # Auth / connection parameters (doc_fragments) - if echo "$CHANGED" | grep -qE '^plugins/doc_fragments/'; then - FRAG_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/doc_fragments/') - if [ -n "$FRAG_CHANGES" ]; then - TRIGGERS="${TRIGGERS}\n- **Doc fragment changes (may affect auth/connection params)**: \`$(echo "$FRAG_CHANGES" | tr '\n' ' ')\`" - fi + # ── Auth / connection parameter changes (doc_fragments) ─────────── + FRAG_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/doc_fragments/' || true) + if [ -n "$FRAG_CHANGES" ]; then + FILES=$(echo "$FRAG_CHANGES" | tr '\n' ' ') + INFORMATIONAL="${INFORMATIONAL}\n- **Auth/connection param changes** (doc_fragments): \`${FILES}\`" fi - # plugin_utils changes (return structure, error taxonomy, auth) - if echo "$CHANGED" | grep -qE '^plugins/plugin_utils/'; then - UTILS_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/plugin_utils/' | grep -v '__pycache__') - if [ -n "$UTILS_CHANGES" ]; then - TRIGGERS="${TRIGGERS}\n- **plugin_utils changes (may affect return structure or auth)**: \`$(echo "$UTILS_CHANGES" | tr '\n' ' ')\`" - fi + # ── plugin_utils changes (return structure, error taxonomy, auth) ── + UTILS_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/plugin_utils/' \ + | grep -v '__pycache__' || true) + if [ -n "$UTILS_CHANGES" ]; then + FILES=$(echo "$UTILS_CHANGES" | tr '\n' ' ') + INFORMATIONAL="${INFORMATIONAL}\n- **plugin_utils changes** (may affect return structure or shared auth): \`${FILES}\`" + fi + + # ── Lookup plugin changes ───────────────────────────────────────── + LOOKUP_CHANGES=$(echo "$CHANGED" | grep -E '^plugins/lookup/' || true) + if [ -n "$LOOKUP_CHANGES" ]; then + FILES=$(echo "$LOOKUP_CHANGES" | tr '\n' ' ') + INFORMATIONAL="${INFORMATIONAL}\n- **Lookup plugin changes**: \`${FILES}\`" + fi + + # ── RETURN documentation changes (content-level) ───────────────── + RETURN_CHANGES=$(grep -n '^[+-].*RETURN\s*=' /tmp/plugin_diff.txt \ + | grep -v '^+++' | grep -v '^---' || true) + if [ -n "$RETURN_CHANGES" ]; then + INFORMATIONAL="${INFORMATIONAL}\n- **RETURN documentation changed** — verify return value structure is backward compatible" + fi + + # ── Deprecations added (content-level: new deprecated: blocks) ──── + DEPRECATIONS=$(grep -n '^+\s*deprecated:' /tmp/plugin_diff.txt \ + | grep -v '^+++' || true) + if [ -n "$DEPRECATIONS" ]; then + BREAKING="${BREAKING}\n- **Deprecations added** — parameters or plugins marked \`deprecated:\`" + fi + + # ── Deleted modules or action plugins (removed files) ───────────── + DELETED_FILES=$(git diff --name-only --diff-filter=D ${BASE}...HEAD \ + -- plugins/modules/ plugins/action/ 2>/dev/null \ + | grep -v '__pycache__' || true) + if [ -n "$DELETED_FILES" ]; then + FILES=$(echo "$DELETED_FILES" | tr '\n' ' ') + BREAKING="${BREAKING}\n- **Module/plugin removed**: \`${FILES}\` — this is a breaking change" fi - if [ -n "$TRIGGERS" ]; then - echo "triggered=true" >> $GITHUB_OUTPUT - echo "trigger_list<> $GITHUB_OUTPUT - printf "$TRIGGERS" >> $GITHUB_OUTPUT - echo "" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT + # ── Potential parameter removals (minus lines in DOCUMENTATION) ─── + # Looks for removed option keys in DOCUMENTATION strings + REMOVED_OPTIONS=$(grep -n '^-\s\+[a-z_]\+:\s*$' /tmp/plugin_diff.txt \ + | grep -v '^---' | head -10 || true) + if [ -n "$REMOVED_OPTIONS" ]; then + BREAKING="${BREAKING}\n- **Possible parameter removal** — verify no existing parameters were deleted from DOCUMENTATION" + fi + + # ── Outputs ─────────────────────────────────────────────────────── + if [ -n "$BREAKING" ]; then + echo "triggered=true" >> $GITHUB_OUTPUT + echo "is_breaking=true" >> $GITHUB_OUTPUT + elif [ -n "$INFORMATIONAL" ]; then + echo "triggered=true" >> $GITHUB_OUTPUT + echo "is_breaking=false" >> $GITHUB_OUTPUT else - echo "triggered=false" >> $GITHUB_OUTPUT + echo "triggered=false" >> $GITHUB_OUTPUT + echo "is_breaking=false" >> $GITHUB_OUTPUT fi - - name: Post CasC reminder comment - if: steps.detect-triggers.outputs.triggered == 'true' - uses: actions/github-script@v6 + { + echo "informational_list<> $GITHUB_OUTPUT + + { + echo "breaking_list<> $GITHUB_OUTPUT + + - name: Check PR description for CasC acknowledgement + id: pr-ack + if: steps.detect.outputs.triggered == 'true' env: - TRIGGER_LIST: ${{ steps.detect-triggers.outputs.trigger_list }} + PR_BODY: ${{ github.event.pull_request.body }} + run: | + # Author has checked "Not applicable" — no CasC impact + if echo "$PR_BODY" | grep -qF '[x] Not applicable — this change does not affect the CasC-monitored surface'; then + echo "acknowledged=true" >> $GITHUB_OUTPUT + echo "ack_reason=not-applicable" >> $GITHUB_OUTPUT + # Author has confirmed Jira ticket created + elif echo "$PR_BODY" | grep -qF '[x] CasC Jira ticket created'; then + echo "acknowledged=true" >> $GITHUB_OUTPUT + echo "ack_reason=jira-created" >> $GITHUB_OUTPUT + else + echo "acknowledged=false" >> $GITHUB_OUTPUT + echo "ack_reason=none" >> $GITHUB_OUTPUT + fi + + - name: Post or update CasC reminder comment + if: steps.detect.outputs.triggered == 'true' + uses: actions/github-script@v7 + env: + INFORMATIONAL_LIST: ${{ steps.detect.outputs.informational_list }} + BREAKING_LIST: ${{ steps.detect.outputs.breaking_list }} + IS_BREAKING: ${{ steps.detect.outputs.is_breaking }} + ACKNOWLEDGED: ${{ steps.pr-ack.outputs.acknowledged }} + ACK_REASON: ${{ steps.pr-ack.outputs.ack_reason }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const triggerList = process.env.TRIGGER_LIST; - const body = [ - "## CasC Notification", - "", - "This PR touches areas that may affect the **CasC collections** (e.g. infra.aap_configuration).", - "", - "**Detected changes in CasC-monitored areas:**", - triggerList, - "", - "Please tag the CasC collections team in this PR so they are aware of the change.", - "", - "> This comment is posted automatically and does not block merge.", - ].join("\n"); - - // Only post once per PR — check if comment already exists + const isBreaking = process.env.IS_BREAKING === 'true'; + const acknowledged = process.env.ACKNOWLEDGED === 'true'; + const ackReason = process.env.ACK_REASON; + const infoList = process.env.INFORMATIONAL_LIST.trim(); + const breakingList = process.env.BREAKING_LIST.trim(); + + let statusEmoji, statusLine; + if (acknowledged && ackReason === 'not-applicable') { + statusEmoji = '✅'; + statusLine = 'Marked **not applicable** in PR description — no CasC action needed.'; + } else if (acknowledged) { + statusEmoji = '✅'; + statusLine = 'CasC Jira ticket confirmed in PR description.'; + } else if (isBreaking) { + statusEmoji = '🔴'; + statusLine = '**Breaking change detected. Complete the CasC section in the PR description — this check will fail until acknowledged.**'; + } else { + statusEmoji = '🟡'; + statusLine = 'Complete the CasC section in the PR description before merging.'; + } + + let body = `## CasC Notification ${statusEmoji}\n\n`; + + if (infoList) { + body += `**Changes in CasC-monitored areas:**\n${infoList}\n\n`; + } + if (breakingList) { + body += `**⚠️ Potential breaking changes:**\n${breakingList}\n\n`; + } + + body += `${statusLine}\n\n`; + if (!acknowledged) { + body += `**Required steps** (unless marking "Not applicable"):\n`; + body += `1. Create a Jira ticket in the CasC project — label: \`ansible.platform\`\n`; + body += `2. Tag the CasC team in this PR\n`; + body += `3. For breaking changes: include a migration guide in the PR description\n`; + body += `4. Check the boxes in the **CasC Notification** section of the PR description\n\n`; + } + body += `> See [CONTRIBUTING.md](CONTRIBUTING.md#casc-notification) for the full procedure.`; + + // Update existing comment rather than posting a duplicate const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, @@ -116,11 +234,18 @@ jobs: }); const existing = comments.data.find(c => - c.body.includes("CasC Notification Required") && - c.user.login === "github-actions[bot]" + c.body.startsWith('## CasC Notification') && + c.user.login === 'github-actions[bot]' ); - if (!existing) { + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body: body, + }); + } else { await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, @@ -128,4 +253,35 @@ jobs: body: body, }); } -... + + - name: Fail for unacknowledged breaking changes + if: | + steps.detect.outputs.triggered == 'true' && + steps.detect.outputs.is_breaking == 'true' && + steps.pr-ack.outputs.acknowledged != 'true' + run: | + echo "::error title=CasC Notification Required::Breaking change detected." + echo "::error::Complete the CasC Notification section of the PR description:" + echo "::error:: 1. Create a CasC Jira ticket (label: ansible.platform)" + echo "::error:: 2. Tag the CasC team in this PR" + echo "::error:: 3. Check [x] CasC Jira ticket created OR [x] Not applicable" + echo "::error::See CONTRIBUTING.md §CasC Notification for the full procedure." + exit 1 + + - name: CasC check passed + if: | + steps.detect.outputs.triggered == 'false' || + steps.detect.outputs.is_breaking == 'false' || + steps.pr-ack.outputs.acknowledged == 'true' + run: | + TRIGGERED="${{ steps.detect.outputs.triggered }}" + BREAKING="${{ steps.detect.outputs.is_breaking }}" + ACK="${{ steps.pr-ack.outputs.acknowledged }}" + + if [ "$TRIGGERED" = "false" ]; then + echo "No CasC-monitored areas touched. Check passed." + elif [ "$BREAKING" = "true" ] && [ "$ACK" = "true" ]; then + echo "Breaking change — CasC notification acknowledged in PR description. Check passed." + else + echo "CasC-monitored areas touched (non-breaking). Reminder posted. Check passed." + fi diff --git a/CODEOWNERS b/CODEOWNERS index cac0b831..84bdaf05 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,5 +1,183 @@ -@jillr -@sean-m-sullivan -@dafmendo -@chuckbrant -@djdanielsson +# ============================================================================= +# CODEOWNERS — ansible.platform +# Governance: ANSTRAT-1640 P2 §1.6 (domain-based ownership) +# ============================================================================= +# +# Three-tier model: +# Tier 1 — TSC: @ansible/tsc-platform-collections +# Tier 2 — Stewards: @ansible/platform-collection-stewards +# Tier 3 — PDTs: @ansible/gateway-pdt @ansible/controller-pdt +# @ansible/eda-pdt @ansible/hub-pdt +# +# One-time GitHub team setup (ansible org): +# platform-collection-stewards → jillr, sean-m-sullivan, dafmendo, chuckbrant, djdanielsson +# gateway-pdt → Gateway component PDT members +# controller-pdt → Controller (AWX) PDT members +# eda-pdt → EDA PDT members +# hub-pdt → Hub/Galaxy PDT members +# tsc-platform-collections → TSC members (overlap with Stewards is fine) +# +# Rules: +# - At least ONE owner per domain must approve before merge. +# - Cross-domain PRs need approval from ALL domains touched. +# - Stewards may not self-approve on behalf of a PDT; PDTs may not self-approve +# shared-infra changes — the path rules enforce separation. +# - For DAB RBAC modules: ALL affected PDTs must approve (see below). +# +# See ANSTRAT-1640 P2 §1.5–1.6 and CONTRIBUTING.md §Cross-domain modules. +# ============================================================================= + +# ── Blanket fallback: Stewards own everything not explicitly matched ────────── +* @ansible/platform-collection-stewards + +# ── Shared infrastructure (Stewards) ───────────────────────────────────────── +# Collection-wide; changes affect all PDTs. +/plugins/plugin_utils/ @ansible/platform-collection-stewards +/plugins/connection/ @ansible/platform-collection-stewards +/plugins/doc_fragments/ @ansible/platform-collection-stewards +/plugins/lookup/ @ansible/platform-collection-stewards +/.github/ @ansible/platform-collection-stewards +/docs/ @ansible/platform-collection-stewards +/changelogs/ @ansible/platform-collection-stewards +CHANGELOG.rst @ansible/platform-collection-stewards +galaxy.yml @ansible/platform-collection-stewards +pyproject.toml @ansible/platform-collection-stewards +tox.ini @ansible/platform-collection-stewards +tox-ansible.ini @ansible/platform-collection-stewards +Makefile @ansible/platform-collection-stewards +conftest.py @ansible/platform-collection-stewards +/requirements/ @ansible/platform-collection-stewards + +# ── Collection routing — FQCN stability (Stewards + TSC) ───────────────────── +# Renames/redirects here can silently break existing playbooks. +/meta/ @ansible/platform-collection-stewards @ansible/tsc-platform-collections + +# ── Tests (Stewards triage; PDT owns domain failures) ──────────────────────── +/tests/unit/ @ansible/platform-collection-stewards +/tests/integration/ @ansible/platform-collection-stewards +/extensions/molecule/ @ansible/platform-collection-stewards +/tests/e2e/ @ansible/platform-collection-stewards + +# ============================================================================= +# MODULE OWNERSHIP +# +# Two categories: +# A. Gateway-only modules — API contract owned by Gateway PDT +# B. DAB RBAC modules — API contract owned by django-ansible-base (shared); +# behavior affects ALL services (Controller/EDA/Hub) +# +# Per ANSTRAT-1640 P2 §1.5: "Ownership follows the source of the API." +# DAB RBAC endpoints are proxied through Gateway but sourced from DAB — +# their behavior defines RBAC across Controller, EDA, and Hub. +# Changes to DAB RBAC modules MUST be reviewed by all affected PDTs. +# ============================================================================= + +# ── A. Gateway-only modules (Gateway PDT + Stewards) ───────────────────────── +# These modules operate against Gateway-native endpoints with no cross-service +# RBAC implications. +/plugins/modules/application.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/application.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/authenticator.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/authenticator.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/authenticator_map.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/authenticator_map.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/authenticator_user.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/authenticator_user.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/ca_certificate.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/ca_certificate.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/http_port.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/http_port.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/route.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/route.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/service.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/service.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/service_cluster.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/service_cluster.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/service_key.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/service_key.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/service_node.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/service_node.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/service_type.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/service_type.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/token.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/token.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/modules/ui_plugin_route.py @ansible/gateway-pdt @ansible/platform-collection-stewards +/plugins/action/ui_plugin_route.py @ansible/gateway-pdt @ansible/platform-collection-stewards + +# ── B. DAB RBAC modules (ALL PDTs — cross-service impact) ──────────────────── +# +# These modules manage organizations, teams, users, and role assignments via +# django-ansible-base (DAB) RBAC endpoints exposed through Gateway. +# Their behavior defines access control INSIDE Controller, EDA, and Hub: +# +# organization — orgs scope resources in Controller, EDA, Hub +# team — teams govern RBAC in Controller, EDA, Hub +# user — users span all services +# role_definition — content_type values are service-specific (awx.*, eda.*, galaxy.*) +# role_team_assignment — assigns team roles that take effect inside Controller/EDA/Hub +# role_user_assignment — assigns user roles that take effect inside Controller/EDA/Hub +# +# A change that looks like a Gateway-side API tweak can silently break +# Controller inventory permissions, EDA rulebook RBAC, or Hub namespace access. +# ALL affected PDTs must review before merge. +# +/plugins/modules/organization.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/organization.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/organization.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +/plugins/modules/team.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/team.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/team.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +/plugins/modules/user.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/user.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/user.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +# role_definition: content_type field references service-specific permission +# namespaces (awx.*, eda.*, galaxy.*). Permissions changes affect which +# actions users/teams can perform inside each service. +/plugins/modules/role_definition.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/role_definition.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/role_definition.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +# role_team_assignment / role_user_assignment: assigns team/user roles (including +# Controller-scoped ones like "Organization Inventory Admin") across services. +# A logic error here can grant or silently drop RBAC in Controller/EDA/Hub. +/plugins/modules/role_team_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/role_team_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/role_team_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +/plugins/modules/role_user_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/action/role_user_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt +/plugins/plugin_utils/api/v1/role_user_assignment.py @ansible/platform-collection-stewards @ansible/gateway-pdt @ansible/controller-pdt @ansible/eda-pdt @ansible/hub-pdt + +# ── Platform-wide modules (Gateway PDT + Stewards + TSC) ───────────────────── +# feature_flag and settings affect platform-wide behaviour across all services. +# TSC oversight required on top of Gateway PDT + Stewards. +/plugins/modules/feature_flag.py @ansible/gateway-pdt @ansible/platform-collection-stewards @ansible/tsc-platform-collections +/plugins/action/feature_flag.py @ansible/gateway-pdt @ansible/platform-collection-stewards @ansible/tsc-platform-collections +/plugins/modules/settings.py @ansible/gateway-pdt @ansible/platform-collection-stewards @ansible/tsc-platform-collections +/plugins/action/settings.py @ansible/gateway-pdt @ansible/platform-collection-stewards @ansible/tsc-platform-collections + +# ── Gateway integration test targets ───────────────────────────────────────── +/tests/integration/targets/ @ansible/gateway-pdt @ansible/platform-collection-stewards + +# ── Examples ───────────────────────────────────────────────────────────────── +/examples/ @ansible/platform-collection-stewards + +# ============================================================================= +# Future PDT lines — uncomment when Controller/EDA/Hub modules land +# (these would be Gateway-only in their own namespace, NOT DAB RBAC) +# ============================================================================= + +# Controller-native modules (AWX API, not DAB RBAC) +# /plugins/modules/controller_*.py @ansible/controller-pdt @ansible/platform-collection-stewards +# /plugins/action/controller_*.py @ansible/controller-pdt @ansible/platform-collection-stewards + +# EDA-native modules +# /plugins/modules/eda_*.py @ansible/eda-pdt @ansible/platform-collection-stewards +# /plugins/action/eda_*.py @ansible/eda-pdt @ansible/platform-collection-stewards + +# Hub-native modules +# /plugins/modules/hub_*.py @ansible/hub-pdt @ansible/platform-collection-stewards +# /plugins/action/hub_*.py @ansible/hub-pdt @ansible/platform-collection-stewards diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 798e3947..0800d3af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,24 +1,250 @@ # Contributing to ansible.platform -This collection is distributed through Red Hat Automation Hub and maintained by Red Hat engineering. Contributions come from PDT members and internal stakeholders. +This collection is governed by [ANSTRAT-1640](https://redhat.atlassian.net/browse/ANSTRAT-1640). Contributions come from Gateway, Controller, EDA, and Hub PDTs, coordinated by Collection Stewards under TSC Platform Collections oversight. + +--- + +## Quick start + +| Task | Where to go | +|------|-------------| +| Add a new module | [docs/07-adding-resources.md](docs/07-adding-resources.md) | +| Architecture overview | [docs/01-overview.md](docs/01-overview.md) | +| SDK / action plugin pattern | [docs/02-action-plugin-pattern.md](docs/02-action-plugin-pattern.md) · [docs/03-sdk-architecture.md](docs/03-sdk-architecture.md) | +| Testing | [docs/08-testing-strategy.md](docs/08-testing-strategy.md) | +| Data model and transforms | [docs/04-data-model-transformation.md](docs/04-data-model-transformation.md) | +| Design principles | [docs/05-design-principles.md](docs/05-design-principles.md) | + +--- + +## Governance model + +Three-tier ownership (ANSTRAT-1640 P2): + +- **TSC Platform Collections** — sets direction, arbitrates cross-team disputes, approves SDPs +- **Collection Stewards** — own CI health, releases, triage, quality gatekeeping, collection-wide docs +- **Component PDTs** (Gateway, Controller, EDA, Hub) — own the correctness of their modules and tests + +Ownership is encoded in `CODEOWNERS`. Shared infrastructure (`plugin_utils`, CI, docs) requires Steward approval. Component modules require the relevant PDT. Cross-domain PRs require both. + +--- + +## Definition of Done + +When your work introduces or changes an API or collection behavior, the PR scope **MUST** include all of: + +- [ ] `ansible.platform` module/plugin code updated +- [ ] `DOCUMENTATION` string updated (params, return values, examples) +- [ ] Integration test added or updated, passing in CI +- [ ] All CI checks green (unit, mock, integration — all 3 connection modes) +- [ ] CasC notification completed (if applicable — see below) + +**This is not optional and not a follow-up ticket.** API-changing work that lands without the corresponding collection update is an immediate regression for CasC and downstream users. --- ## Before opening a PR -- Reference a Jira issue in the PR title: `[AAP-XXXXX] Short description` -- Fill the PR template completely -- Apply the `safe to test` label to trigger integration CI -- Minimum **2 approvals** required before merge +1. Reference a Jira issue in the PR title: `[AAP-XXXXX] Short description` +2. Fill the PR template completely — particularly the CasC Notification and Definition of Done sections +3. Apply the `safe to test` label to trigger integration CI (review the diff before adding this label — it grants access to secrets) +4. Minimum **2 approvals** required (at least one from each CODEOWNERS domain touched) + +--- + +## UX standards (P2R5) + +All plugins **MUST** follow these conventions. Do not invent one-off patterns. + +### Parameter naming + +Use parameters from `plugins/doc_fragments/auth.py`. Do not invent new top-level auth params. + +| Parameter | Description | Env var | +|-----------|-------------|---------| +| `aap_hostname` | URL to AAP Gateway | `AAP_HOSTNAME` | +| `aap_username` | Username | `AAP_USERNAME` | +| `aap_password` | Password | `AAP_PASSWORD` | +| `aap_token` | OAuth/API token | `AAP_TOKEN` | +| `aap_validate_certs` | SSL verification | `AAP_VALIDATE_CERTS` | +| `aap_request_timeout` | Request timeout (float, seconds) | `AAP_REQUEST_TIMEOUT` | + +`gateway_*` aliases are provided for backward compatibility. New code must use `aap_*` names. -For new modules, architecture changes, or anything with cross-PDT impact — open a discussion with the Component Lead first. Full workflow details are in [`docs/07-adding-resources.md`](docs/07-adding-resources.md). +Include auth params via: `extends_documentation_fragment: ansible.platform.auth` + +### State values + +Use `extends_documentation_fragment: ansible.platform.state`. State is defined in `plugins/doc_fragments/state.py`. + +| Value | Meaning | +|-------|---------| +| `present` | Create if absent; update to match | +| `absent` | Delete if present | +| `exists` | Assert exists; read but do not modify | +| `enforced` | Create/update; API-defaults all fields not explicitly provided | + +### Return values + +Every plugin **MUST** define a `RETURN` block. The return structure must use the shared shape from `plugin_utils` — do not invent module-specific return formats. + +For idempotent round-trips: output from `state: exists` must be usable as input to `state: present` without modification. + +### Idempotency + +All operations must be idempotent: running the same task twice produces no change on the second run. + +**`$encrypted$` exception:** When a field is returned as `$encrypted$` it cannot be compared for equality. Convention: exclude that field from the idempotency check (treat masked value as "no change"). Document this in the module's `DOCUMENTATION` notes. + +### Error messages + +Use the shared error taxonomy from `plugin_utils`. Provide context: what was expected, what was found, what the user should do. Do not swallow errors silently. --- -## CasC notification +## Coding standards (P1R9–R10) + +Detailed standards are in [docs/02-action-plugin-pattern.md](docs/02-action-plugin-pattern.md). Summary of what CI enforces: -If your change adds a module, alters auth or return values, or deprecates anything — tag the CasC collections team in the PR. The CI check will post a reminder automatically when trigger areas are touched. +- **Type hints** mandatory on all functions (`mypy`) +- **Docstrings** mandatory on all classes and functions (`pydoclint`) +- **Argspecs derived from `DOCUMENTATION`** — no manual duplication +- **Linting**: `ruff` (replaces flake8, black, isort) + +Run before pushing: + +```bash +make lint # ruff + mypy + pydoclint + ansible-lint +make unit # pytest tests/unit/ +``` --- -See [`docs/`](docs/) for architecture, coding standards, testing strategy, and the full module workflow. +## CasC notification (P2R4) + +You **MUST** notify the CasC collections team when your PR: + +| Trigger | Example | +|---------|---------| +| Adds a new module or action plugin | New file in `plugins/modules/` or `plugins/action/` | +| Changes `aap_*` / `gateway_*` auth or connection params | Edit to `plugins/doc_fragments/auth.py` | +| Changes the `RETURN` documentation or actual return structure | Modified `RETURN =` in a plugin | +| Adds a `deprecated:` block to any `DOCUMENTATION` | New deprecation notice | +| Removes or renames a parameter | Backward-incompatible param change | +| Removes a module or plugin | Deleted file in `plugins/modules/` or `plugins/action/` | + +The `casc-notify-check` CI workflow detects these automatically and posts a reminder comment. **For breaking changes it will fail the check until you acknowledge notification in the PR description.** + +### Notification steps + +1. Create a Jira ticket in the CasC project with label `ansible.platform` +2. Tag the CasC collections team in this PR +3. For breaking changes: include a migration guide (or explicitly state no migration path exists) +4. Check the relevant boxes in the PR description **CasC Notification** section + +### SLA and escalation + +CasC will respond within **5 business days**. If no response, escalate to Collection Stewards or TSC. A lack of CasC bandwidth does not block merge indefinitely — Stewards can approve escalation with a follow-up ticket. + +--- + +## Breaking changes policy (Decision 2) + +- Minimum **12-month deprecation window** (unless security or critical bug) +- Add a `deprecated:` block in `DOCUMENTATION` with `removed_in: "X.Y"` and `why:` +- Add a `CHANGELOG` entry under `breaking_changes:` +- CasC team **must approve** before merge (the CasC check fails until acknowledged) +- Provide a migration guide, or explicitly state there is no migration path + +--- + +## FQCN and backward compatibility + +Existing module FQCNs (e.g. `ansible.platform.organization`) must remain stable. **Never rename a module without:** + +1. Adding a redirect in `meta/routing.yml` +2. Marking the old name deprecated per the breaking changes policy above +3. Steward + TSC approval (both are CODEOWNERS on `meta/`) + +--- + +## Review process (P2R3) + +- Minimum **2 reviewers**. CODEOWNERS are auto-assigned. +- Component Leads identify appropriate reviewers if they cannot review themselves. +- Reviewers verify (where CI cannot): cross-component impact, CasC trigger completion, FQCN stability, architectural fit. +- Cross-domain PRs (shared infra + component code) need approval from **both** domains — a single reviewer cannot cover both. + +### SDP and proposal reviews + +Architecture changes, new connection modes, or anything with cross-PDT impact **must** have a proposal or SDP in the [ansible handbook](https://handbook.eng.ansible.com/) before the implementation PR is opened. All PDTs (Controller, Gateway, EDA, Hub) must approve the proposal. Link the handbook PR from your collection PR description. + +--- + +## Cross-domain modules — DAB RBAC (P2 §1.5) + +Several modules in this collection look like Gateway modules but are actually **cross-service**. They call DAB (django-ansible-base) RBAC endpoints that Gateway proxies, but their effect is felt inside Controller, EDA, and Hub. A change that appears to be a minor Gateway-side fix can silently break Controller inventory permissions, EDA rulebook access, or Hub namespace RBAC. + +These modules require **all affected PDT approvals** before merge, not just Gateway PDT + Stewards: + +| Module | Why cross-domain | +|--------|-----------------| +| `organization` | Orgs scope resources in Controller, EDA, and Hub — deleting or renaming an org cascades across all services | +| `team` | Teams govern RBAC inside Controller, EDA, and Hub — changes to membership or team resolution affect access across all services | +| `user` | Users span all services — changes to user creation, deactivation, or attribute handling have multi-service impact | +| `role_definition` | The `content_type` field references service-specific permission namespaces: `awx.*` (Controller), `eda.*` (EDA), `galaxy.*` (Hub) — a bug here grants or denies permissions inside another service | +| `role_team_assignment` | Assigns team roles such as "Organization Inventory Admin" that grant access inside Controller; also assigns EDA and Hub roles — logic errors silently drop or escalate RBAC | +| `role_user_assignment` | Same as `role_team_assignment` but for individual users | + +**If your PR touches any of these six modules:** + +1. CODEOWNERS will require approval from all PDTs (Controller, EDA, Hub, Gateway, Stewards) +2. In the PR description, explicitly state which services are affected and how you tested cross-service RBAC behaviour +3. For `role_definition` changes: list which `content_type` namespaces are affected +4. For assignment changes: confirm idempotency against Controller, EDA, and Hub role endpoints — not just the Gateway list endpoint + +**`feature_flag` and `settings`** are Gateway-only in their API but platform-wide in their effect. They require TSC oversight on top of the normal Gateway PDT + Stewards review. + +--- + +## Dispute resolution + +When PDTs disagree on a PR: escalate to the **TSC Platform Collections**. TSC Chairs coordinate and facilitate resolution. If consensus fails, TSC makes a binding decision. + +--- + +## Testing requirements (P2R9) + +Three layers are required. All must pass before merge. See [docs/08-testing-strategy.md](docs/08-testing-strategy.md) for details. + +| Layer | Runner | Requires | +|-------|--------|----------| +| Unit tests | `pytest tests/unit/` | No network; mocked dependencies | +| Molecule mock tests | `molecule test -s _mock` | Mock Gateway server | +| Integration tests | `ansible-test integration` | Live Gateway in Docker | + +Integration CI uses `pull_request_target` + `safe to test` label. Review the diff before adding the label — it grants CI access to repo secrets. + +--- + +## OpenAPI alignment (P1R13, P2R10) + +New modules should align with the OpenAPI-first strategy (ANSTRAT-1738). The central spec repo is [aap-openapi-specs](https://github.com/ansible-automation-platform/aap-openapi-specs). If your module introduces a non-spec-driven approach, document the deviation in the PR description. The Feature Architect will review for alignment during SDP/proposal approval. + +--- + +## Platform API issues (P1R16) + +Do not work around platform API shortcomings in collection code. If you encounter a missing endpoint, incorrect behavior, or API limitation: + +1. Document the issue (what was expected, what happened, which endpoint) +2. File a Jira blocker against the responsible platform team +3. Reference the blocker in the collection PR — do not merge a workaround + +--- + +## Steward team responsibilities + +Stewards (not PDTs) own: CI pipeline health, collection releases and release notes, triage of incoming issues, quality gatekeeping, and collection-wide documentation. If CI is broken or a release is needed, ping the Stewards. + +PDTs own: correctness of their modules, updating their modules when their component API changes, and fixing component-domain test failures.