feat(chart): Plugins

[adityathebe]

resolves: flanksource/mission-control-chart#268

Summary by CodeRabbit

• New Features

  • Added AWS locations plugin for automatic scraping and mapping of AWS resource location data.
  • Added Kubernetes locations plugin for automatic scraping and mapping of Kubernetes cluster location data.
  • Both plugins can be enabled or disabled independently via values.

• Chores

  • Added top-level image and plugins keys to values.
  • Tightened values schema to reject unknown properties and validate new plugins configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds two Helm templates that render ScrapePlugin resources for AWS and Kubernetes locations when plugin flags are enabled, and introduces a top-level plugins config and image block in values; also adjusts values schema (removes empty required arrays and tightens additionalProperties usage).

Changes

Cohort / File(s)	Summary
Plugin templates chart/templates/plugins/aws-locations.yaml, chart/templates/plugins/k8s.yaml	Add two new Helm templates that conditionally render ScrapePlugin resources when .Values.plugins.enabled and the respective .Values.plugins.locations.aws or .Values.plugins.locations.kubernetes flags are true. aws-locations maps AWS resource types to aliases and location filters (accounts, regions, zones, VPCs, subnets, EC2 instances, EKS) via tag/label/config placeholders. k8s maps Kubernetes resources (Cluster, Namespace, Deployments, StatefulSets, DaemonSets, Jobs, HelmReleases, Nodes) to aliases and location rules, includes cluster/namespace scoping, has(...) checks, providerID/topology parsing for AWS nodes, and withParent handling for HelmRelease.
Values schema and defaults chart/values.schema.json, chart/values.yaml	Add a new top-level plugins object (enabled boolean, locations.aws and locations.kubernetes booleans) and a top-level image block (name, pullPolicy, tag) in values.yaml. values.schema.json removes multiple empty "required": [] entries and introduces the plugins schema; various objects have explicit additionalProperties constraints tightened.
Build helper Makefile	Update values.schema.json target to run helm-schema with an additional -k additionalProperties flag alongside existing options.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'feat(chart): Plugins' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changeset despite being related to plugins.	Use a more specific title that describes the primary change, such as 'feat(chart): Add AWS and Kubernetes location scrape plugins' or similar to clearly indicate what plugins are being added.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR references issue #268 'Scrape plugin for locations not installed' and implements scrape plugins for AWS and Kubernetes locations with proper Helm templating and conditional rendering.
Out of Scope Changes check	✅ Passed	All changes are in-scope: new plugin templates (aws-locations, k8s), schema updates, values configuration, and Makefile adjustments all directly support the plugins feature and location scraping functionality.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/plugins-in-chart

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

chart/templates/plugins/aws-locations.yaml (1)

1-1: Minor: Fix inconsistent spacing in template condition.

Double space between .Values.plugins.enabled and .Values.plugins.locations.aws should be single space for consistency with best practices.

Note: YAMLlint's error about {{- is a false positive—YAMLlint doesn't natively understand Helm template syntax. If the pipeline rejects this, consider adding YAMLlint configuration to exclude template files or add a directive to skip Helm files.

Apply this diff:

-{{- if and .Values.plugins.enabled  .Values.plugins.locations.aws }}
+{{- if and .Values.plugins.enabled .Values.plugins.locations.aws }}

chart/values.schema.json (2)

3-3: Stricter validation with additionalProperties: false applied broadly.

Root-level and nested additionalProperties: false constraints (e.g., line 416 for image) enforce stricter schema validation. This is good for maintainability but verify that image being added to required (line 809) does not break existing deployments or CI pipelines that may not explicitly define the image block (relying on defaults instead).

If image was previously optional, consider whether making it required is necessary or if it should remain optional with a schema-provided default. Run through deployment scenarios to confirm backward compatibility.

Also applies to: 416-416, 809-809

---

3-3: Extensive schema tightening via additionalProperties: false.

Many nested objects now enforce additionalProperties: false, restricting custom/unknown properties. While this improves schema rigor and prevents configuration drift, the scope is broad. Ensure that:

• Existing production Helm values do not contain unexpected properties that would now be rejected
• CI/CD validation tests cover the new stricter schema
• Teams are aware of the validation changes to prevent deployment surprises

Add integration tests or validation checks to confirm that the stricter schema does not break existing or expected chart usage patterns. Consider documenting breaking schema changes in release notes if this is a patch/minor version bump.

Also applies to: 6-6, 12-12, 21-21, 79-79, 82-82, 103-103, 150-150, 276-276, 284-284, 350-350, 369-369, 387-387, 461-461, 551-551, 661-661, 733-733, 759-759, 774-774

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92491f2 and 0fcff97.

📒 Files selected for processing (4)

• chart/templates/plugins/aws-locations.yaml (1 hunks)
• chart/templates/plugins/k8s.yaml (1 hunks)
• chart/values.schema.json (14 hunks)
• chart/values.yaml (2 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

chart/templates/plugins/aws-locations.yaml

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

(syntax)

chart/templates/plugins/k8s.yaml

[error] 2-2: syntax error: expected '', but found ''

(syntax)

🔇 Additional comments (5)

chart/values.yaml (2)

148-167: Image block addition looks good.

The new top-level image block with template-based name and enum-validated pullPolicy aligns well with the schema.

---

608-631: Plugins block properly structured.

Schema annotations are comprehensive, defaults enable both plugins by default for out-of-box functionality, and the structure aligns with template conditionals.

chart/templates/plugins/k8s.yaml (1)

8-90: ScrapePlugin structure and filters are well-designed.

Aliases and locations properly map Kubernetes resource types to discovery schemes. Filter logic (including parent relationships and AWS provider ID detection) is clear and semantically correct.

chart/templates/plugins/aws-locations.yaml (1)

9-9: Verify unusual type syntax for AWS Account alias.

Line 9 defines the type as AWS::::Account (with four colons). This appears inconsistent with other aliases which use AWS::ResourceType pattern (e.g., AWS::AvailabilityZone on line 13). Confirm whether the four-colon syntax is intentional or a typo.

chart/values.schema.json (1)

542-575: Plugins schema properly structured with sensible defaults.

New plugins configuration requires locations object (which contains aws/kubernetes toggles) and enables both by default. Structure aligns with values.yaml and template conditionals. The constraint that enabled is not required is acceptable since the schema provides a default (true), so the field will apply even if omitted.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92491f2 and 0fcff97.

📒 Files selected for processing (4)

• chart/templates/plugins/aws-locations.yaml (1 hunks)
• chart/templates/plugins/k8s.yaml (1 hunks)
• chart/values.schema.json (14 hunks)
• chart/values.yaml (2 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

chart/templates/plugins/k8s.yaml

[error] 2-2: syntax error: expected '', but found ''

(syntax)

chart/templates/plugins/aws-locations.yaml

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

(syntax)

🔇 Additional comments (5)

chart/templates/plugins/k8s.yaml (1)

1-1: The YAMLlint error is a known false positive with Helm templates—verify via helm lint instead.

YAMLlint doesn't handle Helm template syntax correctly; render templates first before linting them. Use helm lint to check for potential issues in Helm charts, which performs checks on structure, syntax, and best practices. The conditional syntax itself is valid Helm: conditional blocks are a standard pattern in Helm for gating resource creation based on values.

Minor style nit: normalize the double space between .Values.plugins.enabled and .Values.plugins.locations.kubernetes to a single space for consistency.

chart/templates/plugins/aws-locations.yaml (1)

1-1: The YAMLlint error is a known false positive with Helm templates—verify via helm lint instead.

YAMLlint doesn't handle Helm template syntax correctly; render templates first before linting. The {{- syntax for whitespace control is valid Helm pattern. Use helm lint to validate that templates can be rendered successfully.

Minor style nit: normalize the double space between .Values.plugins.enabled and .Values.plugins.locations.aws to a single space for consistency across both plugin templates.

chart/values.schema.json (3)

542-575: Plugin schema structure is sound and aligns with template conditionals.

The new plugins object correctly mirrors the template conditions in both k8s.yaml and aws-locations.yaml. Both feature flags (plugins.locations.aws and plugins.locations.kubernetes) default to true and are optional, allowing users to disable specific location plugins while the overall feature is enabled. The required: ["locations"] ensures the nested structure exists while making individual toggles optional.

---

415-445: New top-level image block schema looks correct; verify chart/values.yaml has matching defaults.

The image schema defines name, pullPolicy, and tag with reasonable defaults. Note that the name default contains a Helm template expression ({{.Values.global.imagePrefix}}/config-db), which is valid but worth verifying works correctly with your schema validation tools at deployment time.

---

807-811: Verify template rendering and schema validation against chart/values.yaml.

The schema changes are comprehensive but the critical file chart/values.yaml isn't included in this review. Before merging, ensure:

1. The values file contains defaults matching this schema (especially the new plugins and image blocks).
2. The schema validates successfully with helm lint.
3. helm template renders the new plugin templates without errors when plugins are enabled/disabled.

[adityathebe]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

chart/values.schema.json (1)

529-560: Plugins schema section lacks additionalProperties: false for consistency.

Most section objects in the schema (e.g., ingress, otel, serviceAccount) include "additionalProperties": false, but the new plugins section does not. If the intent is to enforce strict validation, add this constraint for consistency.

     "plugins": {
+      "additionalProperties": false,
       "properties": {
         "enabled": {
           "default": true,
           "title": "enabled",
           "type": "boolean"
         },

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 38c374c and 865a01d.

📒 Files selected for processing (4)

• Makefile (1 hunks)
• chart/templates/plugins/aws-locations.yaml (1 hunks)
• chart/templates/plugins/k8s.yaml (1 hunks)
• chart/values.schema.json (1 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

chart/templates/plugins/aws-locations.yaml

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

(syntax)

chart/templates/plugins/k8s.yaml

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

(syntax)

🔇 Additional comments (2)

Makefile (1)

228-229: LGTM on Makefile change.

The added -r and -f values.yaml flags align with standalone schema generation from the values file. This complements the helm-schema invocation in the chart target.

chart/values.schema.json (1)

556-557: Verify the required: ["locations"] constraint aligns with intended defaults.

The plugins section declares locations as required, meaning it must always be present in values. Given that individual location toggles (aws, kubernetes) already have defaults, verify this enforces the expected validation behavior and doesn't break existing deployments that omit the plugins section entirely.