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
3 changes: 3 additions & 0 deletions config/crd/bases/eda.ansible.com_edas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,9 @@ spec:
type: object
type: array
type: object
activation_job_namespace:
description: Namespace where activation job pods will run. Creates the namespace and grants required permissions automatically.
type: string
Comment on lines +1479 to +1481

Copy link
Copy Markdown

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, check the CRD file to see the field definition and surrounding context
head -n 1450 config/crd/bases/eda.ansible.com_edas.yaml | tail -n 30

Repository: ansible/eda-server-operator

Length of output: 2425


🏁 Script executed:

# Also get more context around the field to understand its full definition
sed -n '1430,1460p' config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 2517


🏁 Script executed:

# Find and examine the template file mentioned in the review
find . -name "service_account.yaml.j2" -type f

Repository: ansible/eda-server-operator

Length of output: 120


🏁 Script executed:

# Search for how activation_job_namespace is used in the codebase
rg "activation_job_namespace" -B 2 -A 2

Repository: ansible/eda-server-operator

Length of output: 3571


🏁 Script executed:

# Check if there are any validation rules defined for this field or similar namespace fields in the CRD
rg "pattern|maxLength|minLength" config/crd/bases/eda.ansible.com_edas.yaml | head -20

Repository: ansible/eda-server-operator

Length of output: 2557


🏁 Script executed:

# Get the exact section of the CRD for activation_job_namespace with surrounding context
sed -n '1440,1450p' config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 991


🏁 Script executed:

# Search for the full definition of activation_job_namespace field in the CRD including any validation
rg -A 10 "activation_job_namespace:" config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 638


🏁 Script executed:

# Check if there are other namespace-type fields in the CRD that might have validation patterns we should follow
rg -B 2 -A 5 "metadata.name|namespace.*name|DNS.*label" config/crd/bases/eda.ansible.com_edas.yaml | head -50

Repository: ansible/eda-server-operator

Length of output: 53


🏁 Script executed:

# Let's verify Kubernetes namespace naming requirements and check if there's any validation happening elsewhere
# Search for any validationRules or CEL validations in the CRD
rg -i "validationRules|x-kubernetes" config/crd/bases/eda.ansible.com_edas.yaml | head -10

Repository: ansible/eda-server-operator

Length of output: 618


🏁 Script executed:

# Check the full context around where activation_job_namespace is defined to be sure there's no validation
sed -n '1435,1455p' config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 1654


🏁 Script executed:

# Get the exact lines around 1441-1443 to see the field definition
sed -n '1440,1450p' config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 991


🏁 Script executed:

# Look for the activation_job_namespace field definition to see if there are any pattern or validation constraints right after type: string
awk '/activation_job_namespace:/{p=1} p{print} /type: string/&&p{if(++count==1) exit}' config/crd/bases/eda.ansible.com_edas.yaml

Repository: ansible/eda-server-operator

Length of output: 284


🏁 Script executed:

# Check if there are any webhook validators or admission controllers in the repo
find . -type f \( -name "*webhook*" -o -name "*validation*" -o -name "*admission*" \) ! -path "*/node_modules/*" ! -path "*/.git/*" | head -20

Repository: ansible/eda-server-operator

Length of output: 53


🏁 Script executed:

# Look at the template more carefully to understand how activation_job_namespace is used
cat roles/common/templates/service_account.yaml.j2 | grep -A 5 -B 5 "activation_job_namespace"

Repository: ansible/eda-server-operator

Length of output: 1452


Add schema validation for activation_job_namespace to enforce Kubernetes namespace naming rules.

This field is rendered directly into Namespace.metadata.name and RBAC namespace fields without validation. Invalid values will cause reconciliation failures at runtime instead of being rejected at admission time. Add a pattern constraint for DNS-1123 subdomain format with maxLength: 253, while allowing empty string for the default behavior.

🤖 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 `@config/crd/bases/eda.ansible.com_edas.yaml` around lines 1441 - 1443, The
activation_job_namespace field lacks input validation, which allows invalid
Kubernetes namespace names to be accepted and cause runtime failures during
reconciliation. Add schema validation constraints to the
activation_job_namespace field definition by including a pattern constraint that
enforces DNS-1123 subdomain format (allowing lowercase letters, numbers, and
hyphens), set maxLength to 253 to match Kubernetes namespace naming limits, and
ensure the pattern allows empty strings to preserve the default behavior when
the field is not specified.

activation_worker:
description: Defines desired state of eda-activation-worker resources
properties:
Expand Down
7 changes: 7 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ kind: Role
metadata:
name: eda-manager-role
rules:
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- create
Comment on lines +7 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Move namespaces resource rules to a ClusterRole with ClusterRoleBinding.

namespaces is cluster-scoped and cannot be authorized through a namespaced Role. This prevents the activation_job_namespace feature from creating namespaces. Create a separate ClusterRole with the namespace permissions and bind it to the operator's service account.

🤖 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 `@config/rbac/role.yaml` around lines 7 - 13, The namespaces resource is
cluster-scoped and cannot be managed through a namespaced Role. Create a
separate ClusterRole (instead of using the existing namespaced Role) with the
apiGroups, resources, and verbs for the namespaces permission block. Then create
a corresponding ClusterRoleBinding that binds this new ClusterRole to the
operator's service account. Remove the namespaces resource rules from the
current namespaced Role definition.

##
## Base operator rules
##
Expand Down
52 changes: 51 additions & 1 deletion roles/common/templates/service_account.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ metadata:
labels:
{{ lookup("template", "../common/templates/labels/common.yaml.j2") | indent(width=4) | trim }}
rules:
- apiGroups: [""] # "" indicates the core API group
{% if not activation_job_namespace %}
- apiGroups: [""]
resources: ["pods", "pods/log", "jobs", "secrets", "services"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
- apiGroups: ["batch", "extensions"]
Expand All @@ -28,6 +29,7 @@ rules:
- apiGroups: ["networking.k8s.io"]
resources: ["ingresses"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
{% endif %}

---
kind: RoleBinding
Expand All @@ -44,3 +46,51 @@ roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: '{{ ansible_operator_meta.name }}'

{% if activation_job_namespace %}
---
apiVersion: v1
kind: Namespace
metadata:
name: '{{ activation_job_namespace }}'
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: '{{ ansible_operator_meta.name }}-job-execution'
namespace: '{{ activation_job_namespace }}'
labels:
{{ lookup("template", "../common/templates/labels/common.yaml.j2") | indent(width=4) | trim }}
rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
- apiGroups: [""]
resources: ["pods/log"]
verbs: ["get"]
- apiGroups: ["batch", "extensions"]
resources: ["jobs"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get", "create", "delete"]
- apiGroups: [""]
resources: ["services"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: '{{ ansible_operator_meta.name }}-job-execution'
namespace: '{{ activation_job_namespace }}'
labels:
{{ lookup("template", "../common/templates/labels/common.yaml.j2") | indent(width=4) | trim }}
subjects:
- kind: ServiceAccount
name: '{{ ansible_operator_meta.name }}'
namespace: '{{ ansible_operator_meta.namespace }}'
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: '{{ ansible_operator_meta.name }}-job-execution'
{% endif %}
2 changes: 2 additions & 0 deletions roles/eda/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ _image_web_version: "{{ lookup('env', 'DEFAULT_EDA_UI_VERSION') or 'main' }}"
# kubernetes.io/arch: amd64
# kubernetes.io/os: linux

activation_job_namespace: ''

websocket_ssl_verify: false

api: {}
Expand Down
4 changes: 4 additions & 0 deletions roles/eda/templates/eda-activation-worker.deployment.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,10 @@ spec:
- name: EDA_EVENT_PERSISTENCE_PGSSLMODE
value: '{{ event_persistence_postgres_sslmode }}'
{% endif %}
{% if activation_job_namespace %}
- name: EDA_ACTIVATION_JOB_NAMESPACE
value: '{{ activation_job_namespace }}'
{% endif %}
{% if combined_activation_worker.resource_requirements is defined %}
resources: {{ combined_activation_worker.resource_requirements }}
{% endif %}
Expand Down
Loading