Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion telco-core/configuration/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
.PHONY: check
check: compare_crs missing_crs
check: compare_crs missing_crs compare_extra_manifests

.PHONY: compare_extra_manifests
compare_extra_manifests:
./compare.sh --check-extra-manifests

.PHONY: compare_crs
compare_crs:
Expand Down
119 changes: 117 additions & 2 deletions telco-core/configuration/compare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,118 @@ sync_cr() {
git diff --cached --stat --exit-code
}

# Install-time MachineConfig CRs live only under telco-core/install/extra-manifests.
# kube-compare-reference holds validation templates; non-templated files must match install.
compare_install_extra_manifests() {
local root fail=0
root="$(git rev-parse --show-toplevel)"
local install="${root}/telco-core/install/extra-manifests"
local kubecmp="${root}/telco-core/configuration/reference-crs-kube-compare"
local -a pairs=(
"control-plane-load-kernel-modules.yaml:optional/other/control-plane-load-kernel-modules.yaml"
"worker-load-kernel-modules.yaml:optional/other/worker-load-kernel-modules.yaml"
"mount_namespace_config_master.yaml:optional/other/mount_namespace_config_master.yaml"
"mount_namespace_config_worker.yaml:optional/other/mount_namespace_config_worker.yaml"
"kdump-master.yaml:optional/other/kdump-master.yaml"
"kdump-worker.yaml:optional/other/kdump-worker.yaml"
"mc_rootless_pods_selinux.yaml:optional/networking/multus/tap_cni/mc_rootless_pods_selinux.yaml"
)
local pair inst ref refpath
for pair in "${pairs[@]}"; do
inst="${pair%%:*}"
ref="${pair##*:}"
refpath="${kubecmp}/${ref}"
# kube-compare uses Go templates for some MCs; install holds rendered reference YAML.
if grep -q '{{' "${refpath}"; then
if ! grep -q 'validateBase64List' "${refpath}"; then
echo "ERROR: expected validateBase64List templating in kube-compare ${ref}" >&2
fail=1
fi
if ! grep -q 'version: 3.2.0' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must use ignition version 3.2.0" >&2
fail=1
fi
if ! grep -q 'path: /etc/modules-load.d/kernel-load.conf' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must configure kernel-load.conf" >&2
Comment on lines +142 to +147

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Harden invariant checks to avoid regex false positives.

On Line 142 and Line 146, grep -q is using regex semantics, so . acts as a wildcard. That can let malformed manifest values pass this check gate.

Proposed fix
-      if ! grep -q 'version: 3.2.0' "${install}/${inst}"; then
+      if ! grep -Eq '^[[:space:]]*version:[[:space:]]*3\.2\.0([[:space:]]|$)' "${install}/${inst}"; then
         echo "ERROR: ${install}/${inst} must use ignition version 3.2.0" >&2
         fail=1
       fi
-      if ! grep -q 'path: /etc/modules-load.d/kernel-load.conf' "${install}/${inst}"; then
+      if ! grep -Eq '^[[:space:]]*path:[[:space:]]*/etc/modules-load\.d/kernel-load\.conf([[:space:]]|$)' "${install}/${inst}"; then
         echo "ERROR: ${install}/${inst} must configure kernel-load.conf" >&2
         fail=1
       fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ! grep -q 'version: 3.2.0' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must use ignition version 3.2.0" >&2
fail=1
fi
if ! grep -q 'path: /etc/modules-load.d/kernel-load.conf' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must configure kernel-load.conf" >&2
if ! grep -Eq '^[[:space:]]*version:[[:space:]]*3\.2\.0([[:space:]]|$)' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must use ignition version 3.2.0" >&2
fail=1
fi
if ! grep -Eq '^[[:space:]]*path:[[:space:]]*/etc/modules-load\.d/kernel-load\.conf([[:space:]]|$)' "${install}/${inst}"; then
echo "ERROR: ${install}/${inst} must configure kernel-load.conf" >&2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@telco-core/configuration/compare.sh` around lines 142 - 147, The grep
commands on lines checking for 'version: 3.2.0' and 'path:
/etc/modules-load.d/kernel-load.conf' are using regex semantics where the dot
(.) character acts as a wildcard matching any character, which allows incorrect
versions or paths to pass validation. Fix this by either using grep -F flag for
fixed string matching instead of regex, or by escaping the dot characters in the
patterns with backslashes (e.g., 3\.2\.0 and
/etc/modules-load\.d/kernel-load\.conf) to ensure they match literal dots only.

fail=1
fi
continue
fi
if ! diff -u "${install}/${inst}" "${refpath}"; then
echo "ERROR: install/extra-manifests/${inst} differs from kube-compare ${ref}" >&2
fail=1
fi
done
local sctp="${install}/sctp_module_mc.yaml"
if ! grep -q '{{' "${kubecmp}/optional/other/sctp_module_mc.yaml"; then
echo "ERROR: expected templating in optional/other/sctp_module_mc.yaml" >&2
fail=1
fi
if ! grep -q 'version: 3.2.0' "${sctp}"; then
echo "ERROR: ${sctp} must use ignition version 3.2.0" >&2
fail=1
fi
if ! grep -q 'source: data:,sctp' "${sctp}"; then
echo "ERROR: ${sctp} must load sctp via data:,sctp" >&2
fail=1
fi
if grep -q 'version: 2.2.0' "${sctp}" || grep -q 'filesystem: root' "${sctp}"; then
echo "ERROR: ${sctp} must not use legacy ignition 2.2.0 / filesystem fields" >&2
fail=1
fi
return $fail
}

# PolicyGenerator manifest paths must stay under telco-core/configuration/ (no ../).
# MCP CRs are duplicated here and must match install/extra-manifests.
compare_install_custom_manifests_mcp() {
local root fail=0 f
root="$(git rev-parse --show-toplevel)"
local install="${root}/telco-core/install/extra-manifests"
local custom="${root}/telco-core/configuration/reference-crs/custom-manifests"
for f in mcp-worker-1.yaml mcp-worker-2.yaml mcp-worker-3.yaml; do
if ! diff -u "${install}/${f}" "${custom}/${f}"; then
echo "ERROR: install/extra-manifests/${f} differs from reference-crs/custom-manifests/${f}" >&2
fail=1
fi
done
return $fail
}

check_no_machineconfig_in_reference_crs() {
local root fail=0 f
root="$(git rev-parse --show-toplevel)"
while IFS= read -r f; do
echo "ERROR: MachineConfig must use install/extra-manifests, not reference-crs: ${f}" >&2
fail=1
done < <(grep -rl '^kind: MachineConfig$' "${root}/telco-core/configuration/reference-crs" 2>/dev/null || true)
return $fail
}

run_extra_manifest_checks() {
local status=0
echo "Checking install/extra-manifests alignment with kube-compare-reference..."
compare_install_extra_manifests || status=1
echo "Checking install/extra-manifests MCPs match reference-crs/custom-manifests..."
compare_install_custom_manifests_mcp || status=1
echo "Checking reference-crs does not contain MachineConfig CRs..."
check_no_machineconfig_in_reference_crs || status=1
if [[ $status -eq 0 ]]; then
echo "extra-manifest checks: OK"
fi
return $status
}

usage() {
echo "$(basename "$0") [--sync] sourceDir renderDir"
echo "$(basename "$0") [--sync] sourceDir renderDir ignoreFile"
echo "$(basename "$0") --check-extra-manifests"
echo
echo "Compares the rendered reference-based CRs to the CRs in the compare directory"
echo "Compares the rendered reference-based CRs to the CRs in the compare directory,"
echo "or validates install/extra-manifests vs kube-compare-reference."
}

DOSYNC=0
CHECK_EXTRA_MANIFESTS=0
for arg in "$@"; do
case "$arg" in
-h | --help)
Expand All @@ -129,8 +234,18 @@ for arg in "$@"; do
DOSYNC=1
shift
;;
--check-extra-manifests)
CHECK_EXTRA_MANIFESTS=1
shift
;;
esac
done

if [[ $CHECK_EXTRA_MANIFESTS == 1 ]]; then
run_extra_manifest_checks
exit $?
fi

SOURCEDIR=$1
if [[ ! -d $SOURCEDIR ]]; then
echo "No such source directory $SOURCEDIR"
Expand Down
9 changes: 2 additions & 7 deletions telco-core/configuration/core-overlay.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,8 @@ policies:
- mirrors:
- example.com/sample-path
source: nomatch.io/sample-path
- path: reference-crs/optional/other/sctp_module_mc.yaml
complianceType: mustonlyhave
- path: reference-crs/optional/other/worker-load-kernel-modules.yaml
complianceType: mustonlyhave
- path: reference-crs/optional/other/control-plane-load-kernel-modules.yaml
complianceType: mustonlyhave
- path: reference-crs/optional/networking/multus/tap_cni/mc_rootless_pods_selinux.yaml
# MachineConfig CRs are applied at install time from telco-core/install/extra-manifests
# and kept in sync at day-N via the Hub extra-manifests policy (see telco-hub ztp-policies).

# Cert-manager configuration (optional component)
# - path: reference-crs/optional/cert-manager/certManagerClusterIssuer.yaml
Expand Down
20 changes: 15 additions & 5 deletions telco-core/configuration/reference-crs-kube-compare/compare_ignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,25 @@ infrastructure.yaml
optional/networking/sriov/SriovNetworkPoolConfig.yaml
required/networking/metallb/service.yaml

# These are examples only. Customer will provide their own
# based on cluster topology and desired MCP partitioning.
custom-manifests/mcp-worker-1.yaml
custom-manifests/mcp-worker-2.yaml
custom-manifests/mcp-worker-3.yaml
# MachineConfig CRs live under telco-core/install/extra-manifests (install + Hub policy).
# kube-compare keeps optional paths for cluster-compare; they are not duplicated in reference-crs.
optional/other/control-plane-load-kernel-modules.yaml
optional/other/worker-load-kernel-modules.yaml
optional/other/sctp_module_mc.yaml
optional/other/mount_namespace_config_master.yaml
optional/other/mount_namespace_config_worker.yaml
optional/other/kdump-master.yaml
optional/other/kdump-worker.yaml
optional/networking/multus/tap_cni/mc_rootless_pods_selinux.yaml

# Certificate Copying CRs
optional/other/server-cert-openshift-config-copy.yaml

# MachineConfigPool CRs for PolicyGenerator (canonical copies under install/extra-manifests).
custom-manifests/mcp-worker-1.yaml
custom-manifests/mcp-worker-2.yaml
custom-manifests/mcp-worker-3.yaml

# This is an object-template-raw used only to validate subscription state
custom-manifests/subscription-validator.yaml

Expand Down
15 changes: 3 additions & 12 deletions telco-core/configuration/reference-crs-kube-compare/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ parts:
- name: optional-networking
anyOf:
- path: optional/networking/networkAttachmentDefinition.yaml
- path: optional/networking/multus/tap_cni/mc_rootless_pods_selinux.yaml
- path: required/networking/metallb/community.yaml
- path: optional/networking/nodeNetworkConfigurationPolicy.yaml
- name: networking-firewall
Expand Down Expand Up @@ -159,17 +158,17 @@ parts:
- name: other
description: |-
https://docs.redhat.com/en/documentation/openshift_container_platform/4.22/html/scalability_and_performance/telco-core-ref-design-specs#node-configuration-crs

Reference MachineConfig CRs are validated from telco-core/install/extra-manifests
via compare.sh --check-extra-manifests, not from reference-crs.
components:
- name: other
anyOf:
- path: optional/other/control-plane-load-kernel-modules.yaml
- path: optional/other/monitoring-config-cm.yaml
config:
perField:
- pathToKey: data."config.yaml"
inlineDiffFunc: capturegroups
- path: optional/other/worker-load-kernel-modules.yaml
- path: optional/other/sctp_module_mc.yaml
- name: precache
anyOf:
- path: optional/other/precache/controlplane-olm-pinned-images.yaml
Expand Down Expand Up @@ -198,14 +197,6 @@ parts:
- name: other
anyOf:
- path: optional/tuning/control-plane-system-reserved.yaml
- name: kdump-configuration
allOf:
- path: optional/other/kdump-master.yaml
- path: optional/other/kdump-worker.yaml
- name: mount-namespace-configuration
allOf:
- path: optional/other/mount_namespace_config_master.yaml
- path: optional/other/mount_namespace_config_worker.yaml
- name: optional-cert-manager
description: |-
Cert-manager operator for automated certificate management
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
---
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
Expand All @@ -14,12 +15,12 @@ spec:
[Unit]
Description=Set SELinux boolean for tap cni plugin
Before=kubelet.service

[Service]
Type=oneshot
ExecStart=/sbin/setsebool container_use_devices=on
RemainAfterExit=true

[Install]
WantedBy=multi-user.target graphical.target
enabled: true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# optional
# count: 1
---
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# optional
# count: 1
---
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
Expand Down
23 changes: 14 additions & 9 deletions telco-core/configuration/reference-crs/custom-manifests/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
# Custom CRs
This directory is a placeholder for additional custom CRs which are
outside the scope of the reference CRs.

## Reference MCPs
The MCP CRs contained here are examples for a multi-mcp cluster and
are used to build policies which accelerate initial rollout of
configuration by increasing maxUnavailable and manipulating the MCP
pause. The MCPs should be created during initial cluster installation
in a paused state with maxUnavailable of 100%.

This directory holds CRs referenced by PolicyGenerator manifests under
`telco-core/configuration/`. Paths must stay under that directory tree because
the PolicyGenerator kustomize plugin rejects manifest paths outside it (no `../`).

## MachineConfigPool examples

Reference `MachineConfigPool` CRs (`mcp-worker-1.yaml` through `mcp-worker-3.yaml`)
are duplicated here for day-N policies (`core-finish`, `core-upgrade`,
`core-upgrade-finish`). The canonical install-time copies live under
`telco-core/install/extra-manifests/` and must remain identical; `compare.sh
--check-extra-manifests` enforces that.

Other custom content (for example `subscription-validator.yaml`) also belongs here.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ spec:
- key: machineconfiguration.openshift.io/role
operator: In
values: [ worker, worker-1 ]
paused: false
maxUnavailable: 1
paused: true
maxUnavailable: 100%
nodeSelector:
matchLabels:
node-role.kubernetes.io/worker-1: ""
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ spec:
- key: machineconfiguration.openshift.io/role
operator: In
values: [ worker, worker-2 ]
paused: false
maxUnavailable: 1
paused: true
maxUnavailable: 100%
nodeSelector:
matchLabels:
node-role.kubernetes.io/worker-2: ""
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ spec:
- key: machineconfiguration.openshift.io/role
operator: In
values: [ worker, worker-3 ]
paused: false
maxUnavailable: 1
paused: true
maxUnavailable: 100%
nodeSelector:
matchLabels:
node-role.kubernetes.io/worker-3: ""

This file was deleted.

Loading
Loading