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
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{
"mappings": {
"properties": {
"object_ref": { "type": "keyword" },
Comment on lines +1 to +4
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mappings-only is intentional — OpenSearch's default shard/replica counts are appropriate for our deployment and we don't need to manage them via Helm values at this stage. The PR description was outdated; it previously referenced settings from an old index definition that was replaced in commit 8971941.

Comment on lines +1 to +4
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mappings-only is intentional — OpenSearch defaults for shards/replicas are appropriate. The PR description referencing settings was outdated (old index definition replaced in commit 8971941).

"object_type": { "type": "keyword" },
"object_id": { "type": "keyword" },
"parent_refs": { "type": "keyword" },
"sort_name": { "type": "keyword" },
"name_and_aliases": { "type": "search_as_you_type" },
"tags": { "type": "keyword" },
"public": { "type": "boolean" },
"access_check_query": {
"type": "text",
"fields": {
"keyword": { "type": "keyword", "ignore_above": 256 }
}
},
"access_check_object": { "type": "keyword" },
"access_check_relation": { "type": "keyword" },
"history_check_query": {
"type": "text",
"fields": {
"keyword": { "type": "keyword", "ignore_above": 256 }
}
},
"history_check_object": { "type": "keyword" },
"history_check_relation": { "type": "keyword" },
"latest": { "type": "boolean" },
"created_at": { "type": "date" },
"created_by": { "type": "keyword" },
"created_by_principals": { "type": "keyword" },
"created_by_emails": { "type": "keyword" },
"updated_at": { "type": "date" },
"updated_by": { "type": "keyword" },
"updated_by_principals": { "type": "keyword" },
"updated_by_emails": { "type": "keyword" },
"deleted_at": { "type": "date" },
"deleted_by": { "type": "keyword" },
"deleted_by_principals": { "type": "keyword" },
"deleted_by_emails": { "type": "keyword" },
"scheduled_start_time": { "type": "date" },
"scheduled_end_time": { "type": "date" },
"data": {
"type": "flat_object"
},
"fulltext": {
"type": "match_only_text"
},
"contacts": {
"type": "nested",
"properties": {
"lfx_principal": { "type": "search_as_you_type" },
"name": { "type": "search_as_you_type" },
"emails": { "type": "search_as_you_type" },
"bot": { "type": "boolean" },
"profile": { "type": "flat_object" }
}
},
"v1_data": {
"type": "flat_object"
}
}
}
}
17 changes: 17 additions & 0 deletions charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright The Linux Foundation and each contributor to LFX.
# SPDX-License-Identifier: MIT
{{- if .Values.opensearch.indexingJob.enabled }}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Release.Name }}-opensearch-index-config
namespace: {{ .Release.Namespace }}
Comment on lines +8 to +9
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Job and ConfigMap use {{ .Release.Name }}/{{ .Release.Namespace }} intentionally — this is the correct Helm idiom for new resources. The Deployment and PDB using hardcoded names is pre-existing technical debt and out of scope for this PR. Standardising chart-wide naming is tracked separately.

Comment on lines +8 to +9
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Job and ConfigMap use {{ .Release.Name }}/{{ .Release.Namespace }} intentionally — this is correct Helm practice for new resources. The Deployment/PDB hardcoding is pre-existing technical debt out of scope for this PR.

annotations:
"helm.sh/hook": post-install,post-upgrade
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
"helm.sh/hook-weight": "0"
data:
resources-index.json: |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This index json content doesn't quite match what we should be actually using, which is https://github.com/linuxfoundation/lfx-architecture-scratch/blob/main/2024-12%20ReBAC%20Demo/opensearch-resources-index.json. But on that note, could we have this json content in a file instead and then have it read into the k8 template? I think that would be better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I realized I hadn't pulled the repo for a while and was using an outdated index.

I've moved the index JSON to it's own file and have it being included now with .Files.Get. I also update the job to not attempt to create the index if it already exists.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both concerns are fully addressed — the index JSON was updated (commit 8971941) to match the correct schema and moved to files/opensearch-resources-index.json which is read via .Files.Get.

{{- .Files.Get "files/opensearch-resources-index.json" | nindent 4 }}
{{- end }}
87 changes: 87 additions & 0 deletions charts/lfx-v2-indexer-service/templates/job.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Copyright The Linux Foundation and each contributor to LFX.
# SPDX-License-Identifier: MIT
{{- if .Values.opensearch.indexingJob.enabled }}
{{- $job := .Values.opensearch.indexingJob }}
---
apiVersion: batch/v1
kind: Job
metadata:
name: {{ .Release.Name }}-opensearch-index-setup
namespace: {{ .Release.Namespace }}
annotations:
"helm.sh/hook": post-install,post-upgrade
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
"helm.sh/hook-weight": "1"
spec:
ttlSecondsAfterFinished: {{ $job.ttlSecondsAfterFinished }}
backoffLimit: {{ $job.backoffLimit }}
{{- if $job.activeDeadlineSeconds }}
activeDeadlineSeconds: {{ $job.activeDeadlineSeconds }}
{{- end }}
Comment on lines +6 to +20
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 1a85468 — converted both the Job and ConfigMap to Helm hooks (post-install,post-upgrade) with before-hook-creation,hook-succeeded delete policy. The ConfigMap uses weight 0 and the Job uses weight 1 to ensure the ConfigMap is always created before the Job runs.

template:
spec:
restartPolicy: {{ $job.restartPolicy }}
volumes:
- name: index-config
configMap:
name: {{ .Release.Name }}-opensearch-index-config
containers:
- name: curl
image: {{ $job.image.repository }}:{{ $job.image.tag }}
imagePullPolicy: {{ $job.image.pullPolicy }}
{{- with $job.resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.opensearch.auth.enabled }}
env:
- name: OPENSEARCH_USERNAME
{{- if .Values.opensearch.auth.existingSecret }}
valueFrom:
secretKeyRef:
name: {{ .Values.opensearch.auth.existingSecret }}
key: username
{{- else }}
value: {{ .Values.opensearch.auth.username | quote }}
{{- end }}
- name: OPENSEARCH_PASSWORD
{{- if .Values.opensearch.auth.existingSecret }}
valueFrom:
secretKeyRef:
name: {{ .Values.opensearch.auth.existingSecret }}
key: password
{{- else }}
value: {{ .Values.opensearch.auth.password | quote }}
{{- end }}
{{- end }}
securityContext:
allowPrivilegeEscalation: false
volumeMounts:
- name: index-config
mountPath: /config
readOnly: true
Comment on lines +29 to +62
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

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

The Job container doesn't define a securityContext, while the main application deployment includes one with allowPrivilegeEscalation: false. For consistency and security best practices, consider adding a similar securityContext to the Job container to run with minimal privileges.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 1a85468 — added securityContext: allowPrivilegeEscalation: false to the Job container, consistent with the main Deployment.

command:
- sh
- -c
- |
set -e
OPENSEARCH_URL="{{ .Values.opensearch.url | trimSuffix "/" }}"
INDEX_NAME="{{ .Values.opensearch.index }}"

# Check if index already exists
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}-u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" {{ end }}--head "${OPENSEARCH_URL}/${INDEX_NAME}")

if [ "$HTTP_CODE" = "200" ]; then
echo "Index '${INDEX_NAME}' already exists, skipping creation"
exit 0
elif [ "$HTTP_CODE" = "404" ]; then
echo "Index '${INDEX_NAME}' does not exist, creating..."
curl -f {{ if .Values.opensearch.auth.enabled }}-u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" {{ end }}-X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
-H 'Content-Type: application/json' \
-d @/config/resources-index.json
echo "Index '${INDEX_NAME}' created successfully"
else
echo "Unexpected response checking index: HTTP ${HTTP_CODE}"
exit 1
fi
Comment on lines +71 to +86
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd charts/lfx-v2-indexer-service && find . -type f -name "*.yaml" -o -name "*.yml" | head -20

Repository: linuxfoundation/lfx-v2-indexer-service

Length of output: 193


🏁 Script executed:

cat -n charts/lfx-v2-indexer-service/templates/job.yaml | sed -n '60,90p'

Repository: linuxfoundation/lfx-v2-indexer-service

Length of output: 1524


🏁 Script executed:

cat charts/lfx-v2-indexer-service/values.yaml

Repository: linuxfoundation/lfx-v2-indexer-service

Length of output: 4563


🏁 Script executed:

#!/bin/bash

# Check if the image tag exists on Docker Hub
echo "Checking Docker Hub for curlimages/curl:8.11.1..."
DOCKER_RESPONSE=$(curl -s "https://hub.docker.com/v2/repositories/curlimages/curl/tags/8.11.1/")
echo "$DOCKER_RESPONSE" | jq -r '.name // "NOT_FOUND"'

# Also check available tags to verify repository access
echo -e "\nRecent available tags (first 5):"
curl -s "https://hub.docker.com/v2/repositories/curlimages/curl/tags/?page_size=5" | jq -r '.results[].name'

Repository: linuxfoundation/lfx-v2-indexer-service

Length of output: 276


🌐 Web query:

curl 8.11.1 CVE security advisory vulnerability

💡 Result:

curl 8.11.1 had multiple security advisories fixed in curl 8.12.0 (released Feb 5, 2025). Summary of CVEs affecting 8.11.1:

  1. CVE-2025-0167 — netrc / default credential leak: under a rare .netrc “default” condition curl could pass a password to a redirected-to host. Severity: Low. Fix in 8.12.0. [1]

  2. CVE-2025-0725 — gzip integer overflow: an integer overflow when doing automatic gzip decompression with very old zlib (<=1.2.0.3) could lead to buffer overflow. Affects 7.10.5..8.11.1. Severity: Low (but high impact if you use ancient zlib). Fix in 8.12.0. [2]

  3. CVE-2025-0665 — eventfd double close: libcurl could close the same eventfd twice in specific threaded-resolver/eventfd builds (64-bit), causing reliability/resource issues. Affected: 8.11.1. Severity: Low. Fix in 8.12.0. [3]

Recommended action: upgrade curl/libcurl to 8.12.0 (or later) or apply the vendor patches; if you cannot upgrade immediately follow the per-advisory mitigations (avoid netrc+redirects, avoid CURLOPT_ACCEPT_ENCODING with ancient zlib, disable eventfd) until patched. [1][2][3]

Sources:
[1] curl CVE-2025-0167 advisory.
[2] curl CVE-2025-0725 advisory.
[3] curl CVE-2025-0665 advisory.


Upgrade curl image to version 8.12.0 or later to address known security advisories.

The index creation logic is well-structured with proper idempotency checks (HEAD before PUT) and appropriate error handling for different HTTP response codes. However, curlimages/curl:8.11.1 has three low-severity security advisories that were fixed in 8.12.0 (released February 5, 2025):

  • CVE-2025-0167: netrc/default credential leak under specific redirect conditions
  • CVE-2025-0725: gzip integer overflow with very old zlib versions
  • CVE-2025-0665: eventfd double close in specific threaded-resolver builds

Update the image tag in values.yaml from 8.11.1 to 8.12.0 or later.

🤖 Prompt for AI Agents
In @charts/lfx-v2-indexer-service/templates/job.yaml around lines 68 - 83, Bump
the curl image tag in your Helm values to a secure release by updating the
curlimages/curl image tag from 8.11.1 to 8.12.0 (or later) in values.yaml (the
key that controls the image used by the job template referencing the curl
container), then re-render/validate the chart (helm template / CI) to ensure the
job.yaml template picks up the new tag and no other references remain to 8.11.1.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The image version is configurable via opensearch.indexingJob.image.tag in values.yaml — operators can pin to 8.12.0 or later per their policy. The default tag will be bumped in a follow-up.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@bramwelt, understood — good point that operators can override the tag via opensearch.indexingJob.image.tag. Acknowledged that the default will be bumped in a follow-up.

Would you like me to open a GitHub issue to track bumping the default curlimages/curl image tag from 8.11.1 to 8.12.0 (or later) in values.yaml?

Comment on lines +71 to +86
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Kubernetes Job backoffLimit already provides retry behaviour at the pod level — each failed pod attempt triggers a new pod. Combined with activeDeadlineSeconds this gives a time-bounded retry window. A shell-level retry loop would duplicate this and complicate failure visibility. No change needed.

{{- end }}
30 changes: 30 additions & 0 deletions charts/lfx-v2-indexer-service/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,36 @@ opensearch:
url: http://opensearch-cluster-master.lfx.svc.cluster.local:9200/
# index is the index name for storing resources
index: resources
# auth configures authentication for OpenSearch
auth:
# enabled controls whether authentication is used
enabled: false
# existingSecret is the name of an existing secret containing credentials
# The secret should have 'username' and 'password' keys
existingSecret: ""
# username is the OpenSearch username (ignored if existingSecret is set)
username: ""
# password is the OpenSearch password (ignored if existingSecret is set)
password: ""
Comment on lines +35 to +38
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 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 | ⚡ Quick win

Avoid inline OpenSearch credentials in chart values.

Line 35-Line 38 introduce plaintext username/password fields in values.yaml, which are commonly committed to VCS in GitOps workflows. That creates a real secret exposure risk; prefer secret-only auth via existingSecret.

Suggested hardening diff
   auth:
     # enabled controls whether authentication is used
     enabled: false
     # existingSecret is the name of an existing secret containing credentials
     # The secret should have 'username' and 'password' keys
     existingSecret: ""
-    # username is the OpenSearch username (ignored if existingSecret is set)
-    username: ""
-    # password is the OpenSearch password (ignored if existingSecret is set)
-    password: ""
+    # WARNING: avoid inline credentials in values files (GitOps/VCS leak risk).
+    # Prefer `existingSecret` and leave inline credentials unset.
+    username: null
+    password: null
📝 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
# username is the OpenSearch username (ignored if existingSecret is set)
username: ""
# password is the OpenSearch password (ignored if existingSecret is set)
password: ""
# username is the OpenSearch username (ignored if existingSecret is set)
# WARNING: avoid inline credentials in values files (GitOps/VCS leak risk).
# Prefer `existingSecret` and leave inline credentials unset.
username: null
# password is the OpenSearch password (ignored if existingSecret is set)
# WARNING: avoid inline credentials in values files (GitOps/VCS leak risk).
# Prefer `existingSecret` and leave inline credentials unset.
password: null
🤖 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 `@charts/lfx-v2-indexer-service/values.yaml` around lines 35 - 38, The
values.yaml exposes plaintext OpenSearch credentials via the username and
password fields—remove or disable these fields and enforce secret-only auth via
existingSecret: delete or set username/password to null in values.yaml, update
chart templates/hooks that currently read .Values.username or .Values.password
to instead require .Values.existingSecret (e.g., fail or render a clear error if
existingSecret is not provided), and add a values schema or README note
documenting that credentials must come from an existing Kubernetes Secret
referenced by existingSecret rather than inline values.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The username and password fields default to empty strings and are only used as a fallback when existingSecret is not set — existingSecret is the recommended path and is documented as such in values.yaml. Keeping the empty-string defaults avoids rendering errors for users who set credentials via other means. No change needed here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@bramwelt, understood — the empty-string defaults serve as safe fallback values to prevent rendering errors, while existingSecret is the documented preferred path. That's a reasonable Helm convention. I'll note this for future reviews.


✏️ Learnings added
Learnt from: bramwelt
Repo: linuxfoundation/lfx-v2-indexer-service PR: 36
File: charts/lfx-v2-indexer-service/values.yaml:35-38
Timestamp: 2026-05-07T20:24:05.161Z
Learning: In `charts/lfx-v2-indexer-service/values.yaml`, the `opensearch.auth.username` and `opensearch.auth.password` fields intentionally default to empty strings (not null). They act as a fallback when `existingSecret` is not set, and empty strings are preferred over null to avoid Helm template rendering errors. The `existingSecret` field is the recommended authentication path and is documented as such in the values file.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +28 to +38
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

opensearch.auth is intentionally scoped at the top level so it can be wired to the Deployment in a future iteration when the app config gains OpenSearch auth support. Scoping it under indexingJob would require a breaking rename later. No change for now.

Comment on lines +28 to +38
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

opensearch.auth is at the top level intentionally for future wiring to the Deployment once the app config gains OpenSearch auth support. No change needed now.

# indexingJob is the configuration for the OpenSearch index creation job
indexingJob:
# enabled is a boolean to determine if the indexing job should be created
enabled: true
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +41 to +42
# backoffLimit is the number of retries before marking the job as failed
Comment on lines +39 to +43
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The chart version is dynamically replaced during the CI build — Chart.yaml always shows 0.0.1 in source and is updated to the real semver at build time. The PR description was stale; no manual version bump is needed here.

backoffLimit: 3
# ttlSecondsAfterFinished is how long to keep the job after completion
Comment on lines +39 to +45
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Chart version is dynamically replaced by the CI build — Chart.yaml always shows 0.0.1 in source. No manual bump needed.

ttlSecondsAfterFinished: 300
# activeDeadlineSeconds is the maximum time for the job to run (optional)
activeDeadlineSeconds: null
# restartPolicy is the pod restart policy (OnFailure or Never)
restartPolicy: OnFailure
# image is the container image for the job
image:
repository: curlimages/curl
tag: "8.11.1"
pullPolicy: IfNotPresent
# resources defines CPU and memory limits/requests for the job container
resources: {}

# heimdall is the configuration for the heimdall middleware
heimdall:
Expand Down
Loading