Skip to content

Add Helm job for OpenSearch index creation#36

Open
bramwelt wants to merge 11 commits into
mainfrom
bramwelt/indexer-job
Open

Add Helm job for OpenSearch index creation#36
bramwelt wants to merge 11 commits into
mainfrom
bramwelt/indexer-job

Conversation

@bramwelt
Copy link
Copy Markdown
Contributor

@bramwelt bramwelt commented Jan 6, 2026

This pull request adds support for automated OpenSearch index setup in the Helm chart for the LFX V2 Indexer Service. The main changes introduce a Kubernetes Job and ConfigMap to create and configure the OpenSearch index, making the deployment process more streamlined and configurable.

OpenSearch Index Setup Automation

  • Added a new indexingJob configuration section under opensearch in values.yaml to enable or disable automated index setup.
  • Created a new Kubernetes Job manifest (job.yaml) that runs a container to set up the OpenSearch index using the configuration from the ConfigMap.
  • Added a new ConfigMap manifest (indexing-configmap.yaml) that defines the index settings and mappings for OpenSearch, including shard/replica counts and field types.

Issue: LFXV2-1698

Add a Kubernetes Job to the Helm chart that uses cURL to PUT the
OpenSearch index mapping. The index definition is stored in a
ConfigMap and includes mappings for all resource fields used by
the query service.

- Add indexing-configmap.yaml with OpenSearch index JSON
- Add job.yaml that mounts ConfigMap and runs cURL PUT
- Add indexingJob.enabled value (defaults to true)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Copilot AI review requested due to automatic review settings January 6, 2026 20:40
@bramwelt bramwelt requested a review from a team as a code owner January 6, 2026 20:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR introduces OpenSearch indexing job support to the Helm chart by adding configuration values, Kubernetes templates for a ConfigMap and Job, and an OpenSearch index schema. The Job conditionally creates or verifies an index in OpenSearch upon deployment.

Changes

OpenSearch Indexing Job Setup

Layer / File(s) Summary
Configuration Defaults
charts/lfx-v2-indexer-service/values.yaml
opensearch.auth block (enabled, existingSecret, username, password) and opensearch.indexingJob block (enabled, backoffLimit, ttlSecondsAfterFinished, activeDeadlineSeconds, restartPolicy, image settings, resources) are added to values.
Index Schema
charts/lfx-v2-indexer-service/files/opensearch-resources-index.json
OpenSearch index mapping JSON defines typed schema for resources with keyword, date, boolean, nested fields, text search capabilities, and flat objects.
ConfigMap Template
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml
Kubernetes ConfigMap template conditionally renders when opensearch.indexingJob.enabled is true, embedding the index schema JSON from the files directory.
Job Template
charts/lfx-v2-indexer-service/templates/job.yaml
Kubernetes Job template executes a curl-based script that checks if the OpenSearch index exists via HEAD request; if missing (404), creates it via PUT with ConfigMap JSON payload. Includes optional authentication via Secret or direct credentials, retry/deadline settings, and image/resource configuration.

Sequence Diagram

sequenceDiagram
    participant Helm as Helm Deployment
    participant ConfigMap as ConfigMap<br/>(Index Schema)
    participant Job as Kubernetes Job
    participant OpenSearch as OpenSearch API
    
    Helm->>ConfigMap: Create ConfigMap with index schema
    Helm->>Job: Deploy Job (when enabled)
    Job->>Job: Mount ConfigMap at /config
    Job->>OpenSearch: HEAD /index-name (check if exists)
    alt Index exists (HTTP 200)
        OpenSearch-->>Job: 200 OK
        Job->>Job: Log "Index exists"
    else Index missing (HTTP 404)
        OpenSearch-->>Job: 404 Not Found
        Job->>OpenSearch: PUT /index-name with schema from ConfigMap
        OpenSearch-->>Job: 201 Created
        Job->>Job: Log "Index created successfully"
    else Unexpected response
        OpenSearch-->>Job: Other HTTP status
        Job->>Job: Log error and exit with failure
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change—adding a Helm job for OpenSearch index creation—which aligns with the primary objective of automating OpenSearch index setup.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly explains the main changes: automated OpenSearch index setup automation through a new indexingJob configuration, a Kubernetes Job manifest for setup, and a ConfigMap manifest for index configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bramwelt/indexer-job

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request introduces automated OpenSearch index setup functionality to the LFX V2 Indexer Service Helm chart. The implementation adds a Kubernetes Job that creates and configures the OpenSearch index during deployment, streamlining the initial setup process.

Key changes:

  • Added an indexingJob configuration option under opensearch in values.yaml to enable/disable automated index creation
  • Introduced a Kubernetes Job template that uses curl to create the OpenSearch index via REST API
  • Created a ConfigMap template containing index settings and field mappings for the OpenSearch resources index

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File Description
charts/lfx-v2-indexer-service/values.yaml Adds indexingJob.enabled configuration option for controlling automated index setup
charts/lfx-v2-indexer-service/templates/job.yaml New Kubernetes Job for executing OpenSearch index creation using curl
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml New ConfigMap containing index settings and mappings definition
charts/lfx-v2-indexer-service/Chart.yaml Bumps chart version from 0.4.12 to 0.5.0 reflecting the new feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

apiVersion: batch/v1
kind: Job
metadata:
name: opensearch-index-setup
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 name is hardcoded and doesn't include the Release name, which could cause conflicts if multiple releases of this chart are installed in the same namespace. Consider using a template that includes the release name, such as {{ include "lfx-v2-indexer-service.fullname" . }}-index-setup or {{ .Release.Name }}-opensearch-index-setup to ensure uniqueness across releases.

Suggested change
name: opensearch-index-setup
name: {{ include "lfx-v2-indexer-service.fullname" . }}-opensearch-index-setup

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.

Already addressed — both the Job and ConfigMap names use {{ .Release.Name }} for uniqueness across releases.

Comment thread charts/lfx-v2-indexer-service/templates/job.yaml Outdated
Comment thread charts/lfx-v2-indexer-service/templates/job.yaml Outdated
Comment on lines +11 to +12
ttlSecondsAfterFinished: 300
backoffLimit: 3
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'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.

Suggested change
ttlSecondsAfterFinished: 300
backoffLimit: 3
ttlSecondsAfterFinished: {{ .Values.opensearch.indexingJob.ttlSecondsAfterFinished | default 300 }}
backoffLimit: {{ .Values.opensearch.indexingJob.backoffLimit | default 3 }}

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.

Already addressed — ttlSecondsAfterFinished, backoffLimit, and activeDeadlineSeconds are all configurable via opensearch.indexingJob in values.yaml.

apiVersion: v1
kind: ConfigMap
metadata:
name: opensearch-index-config
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 ConfigMap name is hardcoded and doesn't include the Release name, which could cause conflicts if multiple releases of this chart are installed in the same namespace. Consider using a template that includes the release name, such as {{ include "lfx-v2-indexer-service.fullname" . }}-index-config or {{ .Release.Name }}-opensearch-index-config to ensure uniqueness across releases.

Suggested change
name: opensearch-index-config
name: {{ include "lfx-v2-indexer-service.fullname" . }}-opensearch-index-config

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.

Already addressed — the ConfigMap name uses {{ .Release.Name }}-opensearch-index-config.

volumes:
- name: index-config
configMap:
name: opensearch-index-config
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 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.

Suggested change
name: opensearch-index-config
name: {{ .Release.Name }}-opensearch-index-config

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.

Already addressed — the ConfigMap volume reference in the Job matches the ConfigMap name, both using {{ .Release.Name }}.

Comment on lines +15 to +16
"number_of_shards": 1,
"number_of_replicas": 1
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 index settings (number_of_shards and number_of_replicas) are hardcoded in the ConfigMap. Consider making these configurable through values.yaml to allow users to adjust shard and replica counts based on their cluster size and requirements without modifying the ConfigMap template directly.

Suggested change
"number_of_shards": 1,
"number_of_replicas": 1
"number_of_shards": {{ .Values.opensearch.indexingJob.numberOfShards | default 1 }},
"number_of_replicas": {{ .Values.opensearch.indexingJob.numberOfReplicas | default 1 }}

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.

Addressed — the index JSON was moved to files/opensearch-resources-index.json. The shard/replica settings were removed; OpenSearch defaults are appropriate for our deployment.

Comment on lines +21 to +26
- name: curl
image: curlimages/curl:8.11.1
volumeMounts:
- name: index-config
mountPath: /config
readOnly: true
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.

name: opensearch-index-config
containers:
- name: curl
image: curlimages/curl:8.11.1
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 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.

Suggested change
image: curlimages/curl:8.11.1
image: {{ default "curlimages/curl" .Values.opensearch.indexingJob.image }}:{{ default "8.11.1" .Values.opensearch.indexingJob.imageTag }}

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.

Already addressed — the curl image is configurable via opensearch.indexingJob.image.repository and opensearch.indexingJob.image.tag in values.yaml.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI Agents
In @charts/lfx-v2-indexer-service/templates/job.yaml:
- Around line 21-33: The curl invocation in the Job's container command block
should be made to fail on HTTP 4xx/5xx responses; update the multi-line shell
command where curl is run (the command list under the curl container in
job.yaml) to include the --fail (or -f) flag (e.g., change "curl -X PUT ..." to
"curl --fail -X PUT ..." or "curl -f -X PUT ...") so the Kubernetes Job exits
non‑zero on HTTP errors and the failure is surfaced; ensure quoting/escaping in
the same command string is preserved.
🧹 Nitpick comments (2)
charts/lfx-v2-indexer-service/templates/job.yaml (1)

21-26: Consider adding resource limits for the Job container.

While Jobs are typically short-lived, defining resource limits is a best practice to prevent resource exhaustion in the cluster.

🔎 Suggested resource limits
       containers:
         - name: curl
           image: curlimages/curl:8.11.1
+          resources:
+            limits:
+              cpu: "100m"
+              memory: "64Mi"
+            requests:
+              cpu: "50m"
+              memory: "32Mi"
           volumeMounts:
             - name: index-config
               mountPath: /config
               readOnly: true
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml (1)

11-18: Consider making shard and replica counts configurable.

The index settings are hardcoded with 1 shard and 1 replica, which may not be suitable for all deployment environments (development vs. production). Consider making these values configurable through values.yaml.

🔎 Suggested enhancement

In values.yaml, add:

opensearch:
  # ... existing config ...
  indexingJob:
    enabled: true
    shards: 1
    replicas: 1

Then in the ConfigMap:

     {
       "settings": {
         "index": {
-          "number_of_shards": 1,
-          "number_of_replicas": 1
+          "number_of_shards": {{ .Values.opensearch.indexingJob.shards | default 1 }},
+          "number_of_replicas": {{ .Values.opensearch.indexingJob.replicas | default 1 }}
         }
       },
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d380979 and 3512a65.

📒 Files selected for processing (4)
  • charts/lfx-v2-indexer-service/Chart.yaml
  • charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml
  • charts/lfx-v2-indexer-service/templates/job.yaml
  • charts/lfx-v2-indexer-service/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-v2-indexer-service/templates/job.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Agent
  • GitHub Check: MegaLinter
🔇 Additional comments (7)
charts/lfx-v2-indexer-service/Chart.yaml (1)

9-9: LGTM! Appropriate version bump for new features.

The minor version bump from 0.4.12 to 0.5.0 correctly follows semantic versioning for the addition of the new indexing job feature.

charts/lfx-v2-indexer-service/templates/job.yaml (3)

3-3: Static analysis false positive - syntax is valid.

The yamllint error flagging line 3 is a false positive. The {{- if syntax is valid Helm template syntax and will be properly rendered during chart processing.


5-12: Good Job configuration with TTL cleanup.

The Job configuration follows best practices:

  • Short TTL (5 minutes) for automatic cleanup
  • Reasonable backoff limit
  • Appropriate for one-time index setup

30-33: Confirm this is intentional: OpenSearch cluster does not require authentication.

The curl command lacks authentication headers, and no authentication configuration exists in the Helm chart (values.yaml contains no credentials, secrets, or auth settings). Since the OpenSearch URL points to an internal Kubernetes service endpoint, this appears intentional for internal cluster communication. If the deployment environment requires authentication, credentials must be added to the curl command or injected via secrets.

charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml (3)

3-3: Static analysis false positive - syntax is valid.

The yamllint error is a false positive. The {{- if syntax is valid Helm template syntax.


19-64: Well-structured index mappings.

The explicit field type mappings are comprehensive and follow OpenSearch best practices:

  • Appropriate use of keyword type for exact matching (IDs, references)
  • Boolean types for flags
  • search_as_you_type for the name field to support autocomplete functionality

39-40: The search_as_you_type field type is widely supported in OpenSearch v2.x and v3.x (current releases), where it functions as a native autocomplete field type. No specific OpenSearch version constraint is documented in this repository; assuming a supported modern version is in use, this field type is compatible and no action is required.

Comment thread charts/lfx-v2-indexer-service/templates/job.yaml Outdated
Comment thread charts/lfx-v2-indexer-service/values.yaml
Copy link
Copy Markdown
Contributor

@andrest50 andrest50 left a comment

Choose a reason for hiding this comment

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

This looks good! I just left one comment.

name: opensearch-index-config
namespace: {{ .Release.Namespace }}
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.

- Extract index mapping to separate JSON file for better maintainability
- Add idempotent index creation that skips if index already exists
- Change restartPolicy to OnFailure for better retry behavior
- Add proper error handling with set -e and curl -f flags

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml:
- Line 8: The ConfigMap is using a hardcoded name "opensearch-index-config"
which can collide across releases; change the metadata name to use the
release-aware template, e.g. replace the value with {{ include
"lfx-v2-indexer-service.fullname" . }}-opensearch-index-config, and update any
references to that ConfigMap (e.g., the configMapRef/name in the Job that
currently points to "opensearch-index-config") to the same templated name so
both creation and consumption match.

In @charts/lfx-v2-indexer-service/templates/job.yaml:
- Line 8: The Job currently uses a hardcoded name "opensearch-index-setup" which
can collide across releases; update the metadata name to use the chart's
fullname template by replacing the literal name with a templated name that
appends "-opensearch-index-setup", e.g. use the Helm helper include
"lfx-v2-indexer-service.fullname" combined with "-opensearch-index-setup" in the
job metadata name so each release gets a unique job name.
- Line 19: The job's ConfigMap reference currently uses the literal name
"opensearch-index-config" but the chart templates create the ConfigMap with a
release-scoped templated name; update the reference so it uses the same
templated name as the ConfigMap (for example, the chart's fullname template plus
the "-opensearch-index-config" suffix or the .Release.Name-based template used
elsewhere) to ensure the job binds to the correct ConfigMap.
🧹 Nitpick comments (4)
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml (1)

11-12: Index settings (shards, replicas) are not configurable.

The loaded JSON file contains only mappings but no settings section, preventing users from customizing number_of_shards and number_of_replicas for different cluster sizes. Consider either:

  1. Adding a settings section to opensearch-resources-index.json with templated values, or
  2. Constructing the complete index configuration dynamically in this ConfigMap using values from values.yaml.
Example: Add settings section to JSON

In opensearch-resources-index.json, add a settings block at the top level:

{
  "settings": {
    "number_of_shards": {{ .Values.opensearch.indexingJob.numberOfShards | default 1 }},
    "number_of_replicas": {{ .Values.opensearch.indexingJob.numberOfReplicas | default 1 }}
  },
  "mappings": { ... }
}

However, this requires the JSON file to become a template (moved to templates/ with .tpl extension).

charts/lfx-v2-indexer-service/files/opensearch-resources-index.json (2)

1-2: Missing settings section limits index configurability.

The JSON defines only mappings but no settings section. This prevents configuring shard count, replica count, refresh intervals, and analysis settings. Consider adding a settings block to allow operational tuning.

Example settings section
{
  "settings": {
    "number_of_shards": 1,
    "number_of_replicas": 1,
    "refresh_interval": "1s"
  },
  "mappings": {
    ...
  }
}

Note: If you want these to be Helm-configurable, this file would need to become a template (see related comment on indexing-configmap.yaml).


18-18: Consider searchability requirements for email fields.

Email fields (created_by_emails, updated_by_emails, deleted_by_emails) are typed as keyword, which means exact-match only. If you need to support partial email searches (e.g., finding all documents by domain @example.com), consider using text with an email analyzer or a multi-field mapping.

Example multi-field mapping
"created_by_emails": {
  "type": "text",
  "fields": {
    "keyword": {
      "type": "keyword"
    }
  }
}

This allows both full-text search on created_by_emails and exact matching on created_by_emails.keyword.

Also applies to: 22-22, 26-26

charts/lfx-v2-indexer-service/templates/job.yaml (1)

36-36: Add timeouts to curl commands to prevent indefinite hangs.

The curl commands on lines 36 and 43 lack --max-time or --connect-timeout flags, which could cause the Job to hang indefinitely if OpenSearch is unresponsive.

🔎 Proposed fix
               # Check if index already exists
-              HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" -X HEAD "${OPENSEARCH_URL}/${INDEX_NAME}")
+              HTTP_CODE=$(curl --max-time 30 --connect-timeout 10 -s -o /dev/null -w "%{http_code}" -X 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 -X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
+                curl --max-time 30 --connect-timeout 10 -f -X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
                   -H 'Content-Type: application/json' \
                   -d @/config/resources-index.json

Also applies to: 43-43

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3512a65 and 234dc9c.

📒 Files selected for processing (3)
  • charts/lfx-v2-indexer-service/files/opensearch-resources-index.json
  • charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml
  • charts/lfx-v2-indexer-service/templates/job.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

charts/lfx-v2-indexer-service/templates/job.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: MegaLinter
🔇 Additional comments (2)
charts/lfx-v2-indexer-service/templates/job.yaml (1)

31-50: Script logic is well-designed.

The implementation correctly handles:

  • Error propagation with set -e
  • Idempotent index creation with HEAD check
  • Proper HTTP error handling with curl -f
  • Clear error messages for debugging
charts/lfx-v2-indexer-service/files/opensearch-resources-index.json (1)

1-48: The reference schema URL returns a 404 error and cannot be verified.

The reference at https://github.com/linuxfoundation/lfx-architecture-scratch/blob/main/2024-12%20ReBAC%20Demo/opensearch-resources-index.json is no longer accessible. Confirm whether this reference has moved, been deleted, or if an updated reference exists. If available, provide the correct URL to complete the schema validation, or verify the current schema through your project's documentation or schema governance process.

Comment thread charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml Outdated
Comment thread charts/lfx-v2-indexer-service/templates/job.yaml Outdated
Comment thread charts/lfx-v2-indexer-service/templates/job.yaml Outdated
bramwelt and others added 2 commits January 7, 2026 09:19
Ensures unique resource names across multiple Helm releases.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml (1)

12-12: Consider validating file existence.

If the file files/opensearch-resources-index.json is missing, the chart will render with empty content for this ConfigMap entry, potentially causing the Job to fail silently or with confusing errors.

💡 Optional: Add validation with Helm's required function
-    {{ .Files.Get "files/opensearch-resources-index.json" | nindent 4 }}
+    {{ .Files.Get "files/opensearch-resources-index.json" | required "files/opensearch-resources-index.json must exist" | nindent 4 }}

This will fail the chart render early with a clear error message if the file is missing.

charts/lfx-v2-indexer-service/templates/job.yaml (2)

11-12: Consider making Job parameters configurable.

The ttlSecondsAfterFinished and backoffLimit values are hardcoded. Making them configurable would allow users to adjust cleanup timing and retry behavior for their specific environments.

♻️ Make Job parameters configurable via values.yaml
-  ttlSecondsAfterFinished: 300
-  backoffLimit: 3
+  ttlSecondsAfterFinished: {{ .Values.opensearch.indexingJob.ttlSecondsAfterFinished | default 300 }}
+  backoffLimit: {{ .Values.opensearch.indexingJob.backoffLimit | default 3 }}

Then add to values.yaml:

opensearch:
  indexingJob:
    ttlSecondsAfterFinished: 300
    backoffLimit: 3

21-26: Consider adding resource limits.

The container doesn't define CPU/memory requests or limits. While this is a short-lived Job, setting modest resource constraints is good practice for cluster resource management.

💡 Optional: Add resource constraints
         - name: curl
           image: curlimages/curl:8.11.1
+          resources:
+            requests:
+              cpu: 100m
+              memory: 64Mi
+            limits:
+              cpu: 200m
+              memory: 128Mi
           volumeMounts:

Adjust values based on observed usage patterns.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 234dc9c and f4b068d.

📒 Files selected for processing (2)
  • charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml
  • charts/lfx-v2-indexer-service/templates/job.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

charts/lfx-v2-indexer-service/templates/job.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (6)
charts/lfx-v2-indexer-service/templates/indexing-configmap.yaml (2)

3-3: YAMLlint syntax error is a false positive.

The YAMLlint error flagging line 3 is expected—YAMLlint doesn't parse Helm template directives. The {{- if syntax is correct for Helm templates.


3-13: ConfigMap structure looks good.

The conditional rendering, templated naming with .Release.Name prefix, and file loading via .Files.Get are all properly implemented. The ConfigMap name matches the reference in job.yaml line 19.

charts/lfx-v2-indexer-service/templates/job.yaml (4)

3-3: YAMLlint syntax error is a false positive.

Same as in indexing-configmap.yaml—YAMLlint doesn't understand Helm template syntax.


7-9: Job metadata properly configured.

The templated Job name with .Release.Name prefix ensures uniqueness across releases in the same namespace.


16-19: Volume configuration looks good.

The ConfigMap reference matches the templated name from indexing-configmap.yaml line 8.


27-50: Command logic is well-implemented.

The idempotency check via HEAD request (lines 35-36) prevents duplicate index creation, and the conditional logic properly handles all cases:

  • Existing index (200): skip creation
  • Missing index (404): create with curl -f flag for proper error handling
  • Unexpected responses: fail with clear error message

The set -e on line 31 ensures the Job fails on any command error.

bramwelt and others added 2 commits January 7, 2026 10:24
Support optional basic auth for OpenSearch via existingSecret reference
or direct username/password values. Also makes job configuration
(backoffLimit, ttlSecondsAfterFinished, image, resources) configurable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @charts/lfx-v2-indexer-service/templates/job.yaml:
- Around line 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.
🧹 Nitpick comments (1)
charts/lfx-v2-indexer-service/templates/job.yaml (1)

32-66: Consider simplifying the authentication pattern.

The current implementation uses nested conditionals—AUTH_OPTS is defined at shell runtime (line 65) only when auth is enabled, and then template-time conditionals (lines 69, 76) determine whether to use it. While correct, this pattern could be more maintainable.

♻️ Alternative: Define AUTH_OPTS unconditionally in shell script
-          {{- if .Values.opensearch.auth.enabled }}
-          env:
+          env:
+          {{- if .Values.opensearch.auth.enabled }}
             - name: OPENSEARCH_USERNAME
               {{- if .Values.opensearch.auth.existingSecret }}
               valueFrom:
@@ -61,9 +61,9 @@
               set -e
               OPENSEARCH_URL="{{ .Values.opensearch.url | trimSuffix "/" }}"
               INDEX_NAME="{{ .Values.opensearch.index }}"
-              {{- if .Values.opensearch.auth.enabled }}
+              {{ if .Values.opensearch.auth.enabled }}
               AUTH_OPTS="-u ${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}"
-              {{- end }}
+              {{ else }}
+              AUTH_OPTS=""
+              {{ end }}
 
               # Check if index already exists
-              HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ end }}-X HEAD "${OPENSEARCH_URL}/${INDEX_NAME}")
+              HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" ${AUTH_OPTS} -X HEAD "${OPENSEARCH_URL}/${INDEX_NAME}")
 
               if [ "$HTTP_CODE" = "200" ]; then
@@ -73,7 +73,7 @@
               elif [ "$HTTP_CODE" = "404" ]; then
                 echo "Index '${INDEX_NAME}' does not exist, creating..."
-                curl -f {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ end }}-X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
+                curl -f ${AUTH_OPTS} -X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
                   -H 'Content-Type: application/json' \
                   -d @/config/resources-index.json

This eliminates the template-time conditionals in the curl commands by ensuring AUTH_OPTS is always defined (empty string when auth is disabled).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f4b068d and f9577f5.

📒 Files selected for processing (2)
  • charts/lfx-v2-indexer-service/templates/job.yaml
  • charts/lfx-v2-indexer-service/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-v2-indexer-service/templates/job.yaml

[error] 3-3: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (3)
charts/lfx-v2-indexer-service/templates/job.yaml (1)

3-3: The static analysis syntax error is a false positive.

YAMLlint doesn't recognize Helm template directives. The {{- if .Values.opensearch.indexingJob.enabled }} syntax is valid Helm templating and will be processed correctly during chart rendering.

charts/lfx-v2-indexer-service/values.yaml (2)

28-38: Well-structured authentication configuration.

The auth configuration properly supports both the existingSecret pattern (recommended) and direct credentials (fallback). Comments clearly document the precedence and structure.

Note: When using direct credentials (username/password fields), they will be visible in the values file and rendered into Kubernetes resources. For production deployments, strongly prefer the existingSecret approach.


39-57: Review the enabled-by-default behavior for production deployments.

The indexingJob configuration has sensible defaults (backoffLimit: 3, ttlSecondsAfterFinished: 300, restartPolicy: OnFailure). However, enabled: true means the index creation job will run automatically on every chart installation or upgrade.

Consider whether this default behavior is appropriate:

  • Current behavior: Job runs automatically, creating the index if it doesn't exist (idempotent)
  • Alternative: Default to false, requiring explicit opt-in

If the index creation is essential for the service to function, the current default makes sense. Otherwise, you may want to default to false to prevent unexpected job executions.

Additionally, resources: {} means no CPU/memory limits or requests are set. For a short-lived Job this is usually acceptable, but consider adding defaults in resource-constrained environments:

resources:
  requests:
    cpu: "50m"
    memory: "64Mi"
  limits:
    cpu: "100m"
    memory: "128Mi"

Comment on lines +68 to +83
# Check if index already exists
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ end }}-X 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 }}${AUTH_OPTS} {{ 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
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?

Copilot AI review requested due to automatic review settings May 5, 2026 20:31
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@charts/lfx-v2-indexer-service/values.yaml`:
- Around line 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.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9200bf8-421a-4339-93e2-c5f96b443a75

📥 Commits

Reviewing files that changed from the base of the PR and between f9577f5 and 9f97fad.

📒 Files selected for processing (1)
  • charts/lfx-v2-indexer-service/values.yaml

Comment on lines +35 to +38
# username is the OpenSearch username (ignored if existingSecret is set)
username: ""
# password is the OpenSearch password (ignored if existingSecret is set)
password: ""
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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +28 to +38
# 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: ""
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 +39 to +43
# 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
# backoffLimit is the number of retries before marking the job as failed
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.

Comment on lines +6 to +16
apiVersion: batch/v1
kind: Job
metadata:
name: {{ .Release.Name }}-opensearch-index-setup
namespace: {{ .Release.Namespace }}
spec:
ttlSecondsAfterFinished: {{ $job.ttlSecondsAfterFinished }}
backoffLimit: {{ $job.backoffLimit }}
{{- if $job.activeDeadlineSeconds }}
activeDeadlineSeconds: {{ $job.activeDeadlineSeconds }}
{{- end }}
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.

Comment on lines +68 to +83
# Check if index already exists
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ end }}-X 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 }}${AUTH_OPTS} {{ 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
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.

Comment on lines +64 to +76
{{- if .Values.opensearch.auth.enabled }}
AUTH_OPTS="-u ${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}"
{{- end }}

# Check if index already exists
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ end }}-X 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 }}${AUTH_OPTS} {{ end }}-X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
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 — removed the AUTH_OPTS intermediate variable and inlined the credentials directly as -u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" in both curl calls, ensuring proper quoting.

namespace: {{ .Release.Namespace }}
data:
resources-index.json: |
{{ .Files.Get "files/opensearch-resources-index.json" | nindent 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.

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.

Comment on lines +1 to +4
{
"mappings": {
"properties": {
"object_ref": { "type": "keyword" },
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 +8 to +9
name: {{ .Release.Name }}-opensearch-index-config
namespace: {{ .Release.Namespace }}
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.

andrest50 and others added 2 commits May 5, 2026 17:47
-X HEAD causes curl to wait for a response body that never arrives,
hanging the job indefinitely. --head correctly skips body reading.

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Andres Tobon <andrest2455@gmail.com>
- Add access_check_object, access_check_relation
- Add history_check_object, history_check_relation
- Add scheduled_start_time, scheduled_end_time
- Change access_check_query/history_check_query to text
  with keyword sub-field (ignore_above: 256)
- Add doc_values: false, max_shingle_size: 3 to all
  search_as_you_type fields

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Issue: LFXV2-1698
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Copilot AI review requested due to automatic review settings May 7, 2026 19:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Comment on lines +64 to +76
{{- if .Values.opensearch.auth.enabled }}
AUTH_OPTS="-u ${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}"
{{- end }}

# Check if index already exists
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" {{ if .Values.opensearch.auth.enabled }}${AUTH_OPTS} {{ 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 }}${AUTH_OPTS} {{ end }}-X PUT "${OPENSEARCH_URL}/${INDEX_NAME}" \
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 — removed AUTH_OPTS and inlined credentials as -u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" directly in both curl calls.

Comment on lines +8 to +9
name: {{ .Release.Name }}-opensearch-index-config
namespace: {{ .Release.Namespace }}
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.

Comment on lines +28 to +38
# 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: ""
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.

Comment on lines +1 to +4
{
"mappings": {
"properties": {
"object_ref": { "type": "keyword" },
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).

Comment on lines +39 to +45
# 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
# backoffLimit is the number of retries before marking the job as failed
backoffLimit: 3
# ttlSecondsAfterFinished is how long to keep the job after completion
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.

- Convert Job and ConfigMap to Helm hooks (post-install/post-upgrade)
  with before-hook-creation,hook-succeeded delete policy; ConfigMap
  uses weight 0, Job uses weight 1 to ensure ordering
- Add securityContext.allowPrivilegeEscalation: false to Job container
- Remove AUTH_OPTS shell variable; inline credentials directly as
  -u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" to avoid
  unquoted variable expansion
- Fix nindent formatting in ConfigMap to remove leading whitespace
  before Files.Get template call

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Issue: LFXV2-1698
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@bramwelt
Copy link
Copy Markdown
Contributor Author

bramwelt commented May 7, 2026

Review comment resolution summary (commit 1a85468)

Code changes made

Thread Change
6 Added securityContext: allowPrivilegeEscalation: false to Job container
13 Converted Job and ConfigMap to Helm hooks (post-install,post-upgrade) with before-hook-creation,hook-succeeded delete policy; ConfigMap weight 0, Job weight 1
15/15b Removed AUTH_OPTS variable; inlined credentials as -u "${OPENSEARCH_USERNAME}:${OPENSEARCH_PASSWORD}" in both curl calls
16 Fixed ConfigMap nindent — moved {{- to column 0 so JSON renders without extra leading whitespace

Already addressed in previous commits (outdated threads)

Threads 1–5, 7: Job/ConfigMap names, ttl/backoff configurability, curl image configurability, shard/replica settings, and ConfigMap ref — all addressed in earlier commits.

Thread 8 (andrest50): Index JSON updated and moved to files/opensearch-resources-index.json.

No change — design decisions

Thread Rationale
9 (curl CVE) Image version is configurable via opensearch.indexingJob.image.tag; default will be bumped separately
10 (inline credentials) username/password default to "" and are fallback-only; existingSecret is the recommended path
11 (auth scoping) opensearch.auth at top level for future Deployment wiring; scoping under indexingJob would require a breaking rename
12 (chart version) Version is dynamically replaced by CI build; no manual bump needed
14 (retry loop) backoffLimit + activeDeadlineSeconds provide time-bounded retries at the Job level
17 (mappings-only) OpenSearch defaults for shards/replicas are appropriate; PR description was stale
18 (Release.Name vs hardcoded) New resources correctly use {{ .Release.Name }}; Deployment/PDB hardcoding is pre-existing tech debt

Copilot AI review requested due to automatic review settings May 11, 2026 16:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

namespace: {{ .Release.Namespace }}
annotations:
"helm.sh/hook": post-install,post-upgrade
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
Comment on lines +41 to +42
# enabled is a boolean to determine if the indexing job should be created
enabled: true
Comment thread charts/lfx-v2-indexer-service/values.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants