feat(chart): Plugins

[adityathebe]

resolves: flanksource/mission-control-chart#268

Summary by CodeRabbit

• New Features

  • Added AWS and Kubernetes location plugins for automatic scraping and mapping of resource location URIs; each plugin can be toggled via values.

• Chores

  • Added top-level image and plugins keys to chart values; plugins.locations defaults added.
  • Updated schema generation and values schema to include plugins and stricter schema options.
  • Minor fixture update for AWS subnet identifier format.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

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

Changes

Cohort / File(s)	Summary
Plugin templates chart/templates/plugins/aws-locations.yaml, chart/templates/plugins/k8s.yaml	Add Helm templates that conditionally emit configs.flanksource.com/v1 ScrapePlugin manifests when .Values.plugins.enabled and the respective .Values.plugins.locations.{aws,kubernetes} flags are true. aws-locations defines aliases and location rules for AWS resource types and Kubernetes node providerID-derived values; k8s defines aliases and location rules for cluster, namespace, workload, HelmRelease parent relationships, and nodes.
Values schema & defaults chart/values.schema.json, chart/values.yaml	Add top-level plugins object to values and schema (enabled: true, locations.aws: true, locations.kubernetes: true) and add a top-level image block in values.yaml. Remove many empty required: [] occurrences in the schema and require plugins.locations.
Build helper Makefile	Modify values.schema.json generation command to include -k additionalProperties in the helm-schema invocation.
Fixtures fixtures/plugins/aws-locations.yaml	Update fixture to use subnet identifier template {{.id}} instead of {{.name}} for AWS::EC2::Subnet alias values.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(chart): Plugins' accurately reflects the main purpose of the PR: adding plugin support to the Helm chart with new template files, configuration schemas, and values.
Linked Issues check	✅ Passed	The PR adds ScrapePlugin templates for AWS locations and Kubernetes resources, directly addressing the issue to install scrape plugins for locations with conditional rendering and configuration support.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing plugin support: new template files, schema updates, configuration values, Makefile adjustment for schema generation, and fixture updates for testing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/plugins-in-chart

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch feat/plugins-in-chart

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{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.

[github-actions]

Benchstat

Base: 3e1be219b1b936314c2f44a7fb751d1b8ab2bbe2
Head: e47ec2f77ce4b0e944505123706d4b7ea4eaa7e6

📊 1 minor regression(s) (all within 5% threshold)

Benchmark	Base	Head	Change	p-value
BenchSaveResultsUpdateUnchanged/N=1000-4	689.6m	699.4m	+1.42%	0.026

Full benchstat output

goos: linux
goarch: amd64
pkg: github.com/flanksource/config-db/bench
cpu: AMD EPYC 7763 64-Core Processor                
                                         │ bench-base.txt │           bench-head.txt           │
                                         │     sec/op     │    sec/op     vs base              │
BenchSaveResultsSeed/N=1000-4                 631.2m ± 8%   629.1m ± 11%       ~ (p=1.000 n=6)
BenchSaveResultsUpdateUnchanged/N=1000-4      689.6m ± 1%   699.4m ±  1%  +1.42% (p=0.026 n=6)
BenchSaveResultsUpdateChanged/N=1000-4         1.160 ± 1%    1.162 ±  1%       ~ (p=0.485 n=6)
geomean                                       796.2m        799.6m        +0.43%

                                         │ bench-base.txt │           bench-head.txt           │
                                         │      MB/s      │    MB/s     vs base                │
BenchSaveResultsSeed/N=1000-4                0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=6) ¹
BenchSaveResultsUpdateUnchanged/N=1000-4     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=6) ¹
BenchSaveResultsUpdateChanged/N=1000-4       0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=6) ¹
geomean                                                 ²               +0.00%               ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                         │ bench-base.txt │           bench-head.txt           │
                                         │      B/op      │     B/op      vs base              │
BenchSaveResultsSeed/N=1000-4                34.57Mi ± 0%   34.57Mi ± 0%       ~ (p=0.485 n=6)
BenchSaveResultsUpdateUnchanged/N=1000-4     24.70Mi ± 0%   24.67Mi ± 0%       ~ (p=0.310 n=6)
BenchSaveResultsUpdateChanged/N=1000-4       72.20Mi ± 0%   72.23Mi ± 0%       ~ (p=0.394 n=6)
geomean                                      39.50Mi        39.49Mi       -0.02%

                                         │ bench-base.txt │          bench-head.txt           │
                                         │   allocs/op    │  allocs/op   vs base              │
BenchSaveResultsSeed/N=1000-4                 454.0k ± 0%   454.0k ± 0%       ~ (p=0.621 n=6)
BenchSaveResultsUpdateUnchanged/N=1000-4      288.0k ± 0%   288.0k ± 0%       ~ (p=0.221 n=6)
BenchSaveResultsUpdateChanged/N=1000-4        856.6k ± 1%   856.6k ± 1%       ~ (p=0.190 n=6)
geomean                                       482.0k        482.0k       -0.00%