Skip to content

mgmt-23250: Expose AAP configuration from the LZ #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
agonzalezrh merged 3 commits into from
Mar 9, 2026
Merged

mgmt-23250: Expose AAP configuration from the LZ #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c738f9
mgmt-23250: Expose AAP configuration from the LZ
mlorenzofr Mar 4, 2026
b7d8606
Merge branch 'main' into mgmt-23250
agonzalezrh Mar 7, 2026
6c0e0f1
Update 06-day2.yaml
agonzalezrh Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions defaults/vmaas_operators.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,11 @@ aap_bootstrap_job:
tag: latest

aap_license_file: "{{ workingDir }}/config/aap-license.zip"
aap_archive_tmp_dir: "/tmp/aap-config-source"
aap_config_repo: "https://github.com/osac-project/osac-aap.git"
aap_config_version: "main"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

❓ Verification inconclusive

Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd 'vmaas_operators.yaml' | head -n1)"
echo "Inspecting: $file"
rg -n '^\s*aap_config_repo:\s*' "$file"
rg -n '^\s*aap_config_version:\s*' "$file"

python - <<'PY'
import re, pathlib
p = next(pathlib.Path('.').rglob('vmaas_operators.yaml'))
text = p.read_text()
m = re.search(r'^\s*aap_config_version:\s*"?([^"\n]+)"?\s*$', text, re.M)
if not m:
    print("aap_config_version not found")
    raise SystemExit(1)
v = m.group(1).strip()
is_sha = bool(re.fullmatch(r'[0-9a-f]{7,40}', v))
print(f"aap_config_version={v}")
print("immutable_sha=" + str(is_sha))
PY

Repository: rh-ecosystem-edge/enclave

Repository: rh-ecosystem-edge/enclave
Exit code: 0

stdout:

Inspecting: defaults/vmaas_operators.yaml
17:aap_config_repo: "https://github.com/osac-project/osac-aap-config.git"
18:aap_config_version: "main"
aap_config_version=main
immutable_sha=False

Pin aap_config_version to an immutable ref.

Line 18 uses main, which makes archive contents drift over time. For reproducible and auditable deployments, use a tag or commit SHA instead of a branch reference.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@defaults/vmaas_operators.yaml` at line 18, The aap_config_version currently
uses the mutable branch name "main"; update the aap_config_version value to an
immutable ref (a specific tag like "vX.Y.Z" or a commit SHA) so archives don't
drift—locate the aap_config_version entry in defaults/vmaas_operators.yaml and
replace "main" with the chosen tag or SHA and commit the change.

aap_archive_image: "registry.redhat.io/ubi9/nginx-126@sha256:d68c1643e3a1184c38f87391fd1054724d8ed2e14324c090091d6c724dc1c335"
aap_archive_port: 4480
aap_archive_authfile: "{{ workingDir }}/config/pull-secret.json"
aap_archive_dest: "{{ workingDir }}/data/aap-config-archive.tar.gz"
aap_archive_refresh: false # Set to true to force a refresh of the AAP configuration archive
8 changes: 6 additions & 2 deletions playbooks/06-day2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,14 @@
ansible.builtin.include_tasks:
file: tasks/vmaas_validation.yaml
apply:
tags: vmaas
tags:
- vmaas
- aap_archive_service
environment:
KUBECONFIG: "{{ workingDir }}/ocp-cluster/auth/kubeconfig"
tags: vmaas
tags:
- vmaas
- aap_archive_service

- name: Management cluster upgrade
ansible.builtin.include_tasks:
Expand Down
62 changes: 62 additions & 0 deletions playbooks/tasks/aap_archive_service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
- name: "Check if the repository tarball exists"
ansible.builtin.stat:
path: "{{ aap_archive_dest }}"
register: __aap_archive_tarball_exists
failed_when: false
changed_when: false

# If the repository tarball exists, you can force a refresh by setting aap_archive_refresh to true
- name: "Package AAP configuration repository and serve via HTTP"
block:
- name: "Create temporary directory"
ansible.builtin.file:
path: "{{ aap_archive_tmp_dir }}"
state: directory
mode: "0750"

- name: "Clone the configuration repository"
ansible.builtin.git:
repo: "{{ aap_config_repo }}"
dest: "{{ aap_archive_tmp_dir }}"
version: "{{ aap_config_version }}"

- name: "Create tarball of the repository"
ansible.builtin.archive:
path: "{{ aap_archive_tmp_dir }}"
dest: "{{ aap_archive_dest }}"
format: gz
Comment on lines +18 to +28
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

ansible.builtin.archive module exclude .git hidden directories documentation

💡 Result:

To exclude .git (and other “hidden” dot-directories) when creating an archive, use community.general.archive and prefer exclusion_patterns when the dot-directories are inside the paths you’re archiving.

  • exclude_path only excludes items from the top-level path list and its glob expansion (it expects remote absolute paths or globs). If you’re archiving /repo and want to skip /repo/.git, exclude_path won’t help unless .git is part of what your path/glob expands to. [1]
  • exclusion_patterns excludes files/dirs below any archived path using glob-style patterns—this is the right tool to drop .git/ anywhere in the tree. [1]

Example (exclude .git and any dot-directories anywhere under /repo):

- name: Archive repo without VCS + hidden dirs
  community.general.archive:
    path: /repo
    dest: /tmp/repo.tar.gz
    format: gz
    exclusion_patterns:
      - "**/.git"
      - "**/.git/**"
      - "**/.*"
      - "**/.*/*"

Source: exclude_path vs exclusion_patterns parameter behavior and definitions. [1]

References
[1] Ansible Community Docs – community.general.archive module (parameters exclude_path, exclusion_patterns). (docs.ansible.com)

Citations:

🌐 Web query:

ansible.builtin.archive module parameters documentation exclude

💡 Result:

In the archive module, there isn’t a parameter literally named exclude. To exclude content, use:

  • exclude_path: Remote absolute path, glob, or list of paths/globs to exclude from what you specified in path. This is the exclusion mechanism documented for the (older) built-in archive module. [1]
  • If you’re actually using community.general.archive (often used in modern Ansible installs), you also have:
    • exclude_path: excludes specific source paths (after path list/glob expansion). [2]
    • exclusion_patterns: glob-style patterns to exclude files/dirs below any included path (different behavior than exclude_path). [2]

Example (exclude specific directories):

- community.general.archive:
    path:
      - /path/to/foo/*
    dest: /path/file.tar.bz2
    exclude_path:
      - /path/to/foo/bar
      - /path/to/foo/baz

[1][2]

Sources:
[1] Ansible archive module docs (archive_module) – exclude_path definition and examples
[2] community.general.archive module docs – exclude_path vs exclusion_patterns

Exclude .git metadata from the published archive.

Line 25 archives the entire clone directory created at lines 18–23, which includes .git/ and VCS metadata. This bloats the payload and exposes repository history/configuration over HTTP. Add exclude_path to skip it:

Suggested fix 
    - name: "Create tarball of the repository"
      ansible.builtin.archive:
        path: "{{ aap_archive_tmp_dir }}"
        dest: "{{ aap_archive_dest }}"
        format: gz
        exclude_path: "{{ aap_archive_tmp_dir }}/.git"

Alternatively, switch to community.general.archive with exclusion_patterns for more robust pattern matching (e.g., ["**/.git", "**/.git/**"]) if filtering multiple hidden directories.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@playbooks/tasks/aap_archive_service.yaml` around lines 18 - 28, The archive
task ("Create tarball of the repository") is currently packing the whole clone
({{ aap_archive_tmp_dir }}) including .git metadata; update the
ansible.builtin.archive invocation (task name "Create tarball of the
repository") to exclude the repo metadata by adding exclude_path: "{{
aap_archive_tmp_dir }}/.git" so the produced {{ aap_archive_dest }} tarball
omits .git, or if you need pattern matching for multiple hidden dirs swap to
community.general.archive and set exclusion_patterns like
["**/.git","**/.git/**"] on that task.

ignore_errors: "{{ ansible_check_mode }}"

- name: "Set container recreate flag"
ansible.builtin.set_fact:
__aap_archive_container_recreate: true
always:
- name: "Remove temporary directory"
ansible.builtin.file:
path: "{{ aap_archive_tmp_dir }}"
state: absent
no_log: true
when: >
__aap_archive_tarball_exists.stat.exists is false or
__aap_archive_tarball_exists.stat.exists is not defined or
aap_archive_refresh | default(false) | bool

- name: "Run container to publish the AAP configuration"
containers.podman.podman_container:
name: "aap-archive-srv"
image: "{{ aap_archive_image }}"
authfile: "{{ aap_archive_authfile }}"
state: started
restart_policy: always
recreate: "{{ __aap_archive_container_recreate | default(false) | bool }}"
user: "1001"
command: "/usr/libexec/s2i/run"
security_opt:
- no-new-privileges
cap_drop:
- ALL
volumes:
- "{{ aap_archive_dest }}:/opt/app-root/src/aap-config-archive.tar.gz:Z"
publish:
- "{{ aap_archive_port }}:8080"
15 changes: 11 additions & 4 deletions playbooks/tasks/vmaas_validation.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,17 @@
not __aap_license_stat.stat.exists | bool

- name: "Deploy VMaaS"
ansible.builtin.include_tasks:
file: vmaas.yaml
apply:
tags: vmaas
block:
- name: "Deploy AAP configuration archive service in the LZ"
ansible.builtin.include_tasks:
file: aap_archive_service.yaml
apply:
tags: aap_archive_service
Comment on lines +37 to +41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate the relevant files mentioned in the review
git ls-files | grep -E "(vmaas_validation|06-day2)" | head -20

Repository: rh-ecosystem-edge/enclave

Length of output: 129

🏁 Script executed:

# Also search for these files more broadly
find . -name "vmaas_validation.yaml" -o -name "06-day2.yaml" 2>/dev/null

Repository: rh-ecosystem-edge/enclave

Length of output: 133

🌐 Web query:

Ansible include_tasks apply tags filtering behavior with --tags flag

💡 Result:

With ansible.builtin.include_tasks (a dynamic include), tag filtering with ansible-playbook --tags ... is effectively two-stage:

  1. The include_tasks statement is itself a task, so it must match --tags (or be tags: always) to even be executed (and thus to “descend into” the included file). Tags set on the include line apply only to the include task, not automatically to the tasks inside. [1]

  2. After the file is included, tasks inside are still filtered by --tags. If those inner tasks don’t have a matching tag, they’ll be skipped—even though the include ran. [1]

How to make --tags X run all tasks from the included file

Use apply: to propagate keywords (including tags) onto every task inside the included file:

- name: Include my tasks
  ansible.builtin.include_tasks: mytasks.yml
  tags: [mytag]          # ensures the include itself runs when using --tags mytag
  apply:
    tags: [mytag]        # tags every task inside mytasks.yml with mytag

apply is specifically documented as “task keywords (for example tags) that will be applied to the tasks within the include.” [2]

If you want tag inheritance without apply

Prefer the static form import_tasks, where tag inheritance behaves the way people often expect (tags applied to the import apply to tasks in the imported file). Dynamic includes like include_tasks are called out as not using tag inheritance by default. [1]

Sources:
[1] Ansible tags guide (tag inheritance vs dynamic includes, and --tags behavior with dynamic includes)
[2] ansible.builtin.include_tasks module docs (apply: keyword)

🏁 Script executed:

# Read vmaas_validation.yaml around lines 37-41
sed -n '30,50p' ./playbooks/tasks/vmaas_validation.yaml

Repository: rh-ecosystem-edge/enclave

Length of output: 684

🏁 Script executed:

# Read 06-day2.yaml around lines 85-96 to see how this file is invoked
sed -n '80,100p' ./playbooks/06-day2.yaml

Repository: rh-ecosystem-edge/enclave

Length of output: 562

Propagate the vmaas tag into the archive-service include.

Line 41 only applies aap_archive_service to tasks inside aap_archive_service.yaml. When playbooks/06-day2.yaml invokes this file with both tags, --tags vmaas will reach the include task (which inherits the vmaas tag from the parent's apply directive) but skip every child task, so the new prerequisite service never starts in the tagged VMaaS flow.

Suggested change 
     - name: "Deploy AAP configuration archive service in the LZ"
       ansible.builtin.include_tasks:
         file: aap_archive_service.yaml
         apply:
-          tags: aap_archive_service
+          tags:
+            - vmaas
+            - aap_archive_service
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@playbooks/tasks/vmaas_validation.yaml` around lines 37 - 41, The include task
named "Deploy AAP configuration archive service in the LZ" currently applies
only the aap_archive_service tag to the included file; update the include's
apply.tags to propagate the vmaas tag as well (so tasks inside
aap_archive_service.yaml inherit both aap_archive_service and vmaas), e.g.,
change the apply tags for the include of aap_archive_service.yaml to include
both 'aap_archive_service' and 'vmaas'.

- name: "Deploy VMaaS"
ansible.builtin.include_tasks:
file: vmaas.yaml
apply:
tags: vmaas
when:
- vmaas_clusters | length > 0
- __aap_license_stat.stat.exists is defined
Expand Down
1 change: 1 addition & 0 deletions playbooks/templates/aap-config.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ metadata:
namespace: {{ aap_ns | default('ansible-aap') }}
type: Opaque
stringData:
AAP_PROJECT_ARCHIVE_URI: "http://{{ lzBmcIP }}:{{ aap_archive_port }}/aap-config-archive.tar.gz"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a default filter or validation for lzBmcIP.

Unlike aap_archive_port which has a default in defaults/vmaas_operators.yaml, lzBmcIP is user-provided (per config/global.example.yaml). If the user hasn't configured this variable, the template will render with an empty or undefined value, resulting in a malformed URI like http://:4480/....

Consider adding validation or a default filter to fail fast with a clear error message.

🛡️ Proposed fix to add validation 
+{% if lzBmcIP is not defined or lzBmcIP == '' %}
+{{ 'lzBmcIP must be defined in config/global.yaml' | mandatory }}
+{% endif %}
 apiVersion: v1
 kind: Secret
 metadata:

Or use an assertion in the calling playbook before rendering.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@playbooks/templates/aap-config.yaml.j2` at line 8, The
AAP_PROJECT_ARCHIVE_URI template uses the user-provided variable lzBmcIP which
can be empty and produce a malformed URI; add validation or a default to fail
fast by either applying a Jinja default/validation filter to lzBmcIP in the
AAP_PROJECT_ARCHIVE_URI line (so the template raises a clear error or
substitutes a safe default) or add an assertion in the calling playbook to check
lzBmcIP is defined/non-empty before rendering; refer to the
AAP_PROJECT_ARCHIVE_URI template and the lzBmcIP variable when implementing the
change.

REMOTE_CLUSTER_KUBECONFIG_SECRET_NAME: "{{ aap_kubeconfig_secret_name | default('vmaas-cluster-kubeconfig') }}"
Loading