-
Notifications
You must be signed in to change notification settings - Fork 2
Add Helm job for OpenSearch index creation #36
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.
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
3512a65
234dc9c
4f5d8bb
f4b068d
34edd78
f9577f5
9f97fad
6f0124d
8971941
1a85468
0596036
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| { | ||
| "mappings": { | ||
| "properties": { | ||
| "object_ref": { "type": "keyword" }, | ||
|
Comment on lines
+1
to
+4
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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": "keyword" }, | ||
| "history_check_query": { "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" }, | ||
| "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" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||
| # Copyright The Linux Foundation and each contributor to LFX. | ||||||
| # SPDX-License-Identifier: MIT | ||||||
| {{- if .Values.opensearch.indexingJob.enabled }} | ||||||
| --- | ||||||
| apiVersion: v1 | ||||||
| kind: ConfigMap | ||||||
| metadata: | ||||||
| name: opensearch-index-config | ||||||
|
||||||
| name: opensearch-index-config | |
| name: {{ include "lfx-v2-indexer-service.fullname" . }}-opensearch-index-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — the ConfigMap name uses {{ .Release.Name }}-opensearch-index-config.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 1a85468 — moved the template expression to column 0 using {{- trimming so nindent 4 alone handles the indentation, rendering the JSON cleanly without extra leading whitespace.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,51 @@ | ||||||||||
| # Copyright The Linux Foundation and each contributor to LFX. | ||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||
| {{- if .Values.opensearch.indexingJob.enabled }} | ||||||||||
| --- | ||||||||||
| apiVersion: batch/v1 | ||||||||||
| kind: Job | ||||||||||
| metadata: | ||||||||||
| name: opensearch-index-setup | ||||||||||
|
||||||||||
| name: opensearch-index-setup | |
| name: {{ include "lfx-v2-indexer-service.fullname" . }}-opensearch-index-setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — both the Job and ConfigMap names use {{ .Release.Name }} for uniqueness across releases.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Job's ttlSecondsAfterFinished (300 seconds) and backoffLimit (3) are hardcoded. Consider making these values configurable through values.yaml (e.g., opensearch.indexingJob.ttlSecondsAfterFinished and opensearch.indexingJob.backoffLimit) to allow users to adjust cleanup timing and retry behavior based on their operational requirements.
| ttlSecondsAfterFinished: 300 | |
| backoffLimit: 3 | |
| ttlSecondsAfterFinished: {{ .Values.opensearch.indexingJob.ttlSecondsAfterFinished | default 300 }} | |
| backoffLimit: {{ .Values.opensearch.indexingJob.backoffLimit | default 3 }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — ttlSecondsAfterFinished, backoffLimit, and activeDeadlineSeconds are all configurable via opensearch.indexingJob in values.yaml.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ConfigMap reference uses a hardcoded name that should match the ConfigMap resource name. If the ConfigMap name is updated to include the Release name (as recommended), this reference must also be updated to use the same templated name to ensure proper binding.
| name: opensearch-index-config | |
| name: {{ .Release.Name }}-opensearch-index-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — the ConfigMap volume reference in the Job matches the ConfigMap name, both using {{ .Release.Name }}.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The curl image and version are hardcoded in the Job template. Consider making the image repository and tag configurable through values.yaml (e.g., opensearch.indexingJob.image and opensearch.indexingJob.imageTag) to allow users to specify alternative images or pin specific versions according to their organization's policies.
| image: curlimages/curl:8.11.1 | |
| image: {{ default "curlimages/curl" .Values.opensearch.indexingJob.image }}:{{ default "8.11.1" .Values.opensearch.indexingJob.imageTag }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — the curl image is configurable via opensearch.indexingJob.image.repository and opensearch.indexingJob.image.tag in values.yaml.
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd charts/lfx-v2-indexer-service && find . -type f -name "*.yaml" -o -name "*.yml" | head -20Repository: 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.yamlRepository: 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:
-
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]
-
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]
-
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.