Add support for release field in agentless deployment mode#1130
Add support for release field in agentless deployment mode#1130MichelLosier wants to merge 10 commits intoelastic:mainfrom
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/integration/manifest.spec.yml`:
- Around line 298-312: The new YAML schema property
deployment_modes.agentless.release was added without a version patch; add a
version patch that removes this property for spec versions < 3.6.1 so older
manifests cannot accept deployment_modes.agentless.release. Update the
manifest.spec.yml version-patch section (also mirror the change for the related
block noted around the 947-1073 region) to explicitly delete or disallow the
agentless.release key for pre-3.6.1 schemas, ensuring the enum/type entries for
release only exist in the 3.6.1+ schema.
🪄 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: d7107b47-2c92-4504-9934-141a3ba94d17
📒 Files selected for processing (4)
code/go/internal/validator/semantic/validate_deployment_modes.gocode/go/internal/validator/semantic/validate_deployment_modes_test.gospec/changelog.ymlspec/integration/manifest.spec.yml
| release: | ||
| description: > | ||
| The maturity level of the agentless deployment mode for this policy template. | ||
| Only needs to be set when the agentless deployment mode has a different maturity than the package's overall release state. | ||
| Not allowed in single policy template packages where agentless is the only | ||
| deployment mode. | ||
| type: string | ||
| enum: | ||
| - beta | ||
| - rc | ||
| - preview | ||
| examples: | ||
| - beta | ||
| - rc | ||
| - preview |
There was a problem hiding this comment.
Missing version patch for newly introduced agentless.release field.
The new field added at Line 298 is not removed for older spec versions, so pre-3.6.1 manifests can incorrectly accept deployment_modes.agentless.release.
Proposed patch
versions:
+ - before: 3.6.1
+ patch:
+ - op: remove
+ path: "/definitions/deployment_modes/properties/agentless/properties/release"
- before: 3.6.0
patch:
# Support for otelcol and dynamic_signal_types in integration packages.
- op: add
path: "/properties/policy_templates/items/properties/inputs/items/properties/type"
value:
not:
const: otelcolAs per coding guidelines: "Version patches enable backward compatibility by removing features from older spec versions."
Also applies to: 947-1073
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/integration/manifest.spec.yml` around lines 298 - 312, The new YAML
schema property deployment_modes.agentless.release was added without a version
patch; add a version patch that removes this property for spec versions < 3.6.1
so older manifests cannot accept deployment_modes.agentless.release. Update the
manifest.spec.yml version-patch section (also mirror the change for the related
block noted around the 947-1073 region) to explicitly delete or disallow the
agentless.release key for pre-3.6.1 schemas, ensuring the enum/type entries for
release only exist in the 3.6.1+ schema.
There was a problem hiding this comment.
Given the issue we are trying to support, I think its permissible to allow this addition to backport to older spec versions. It's a non-breaking change (to my thinking), and allowing it wont force other packages to adopt latest spec to use it. WDYT @elastic/ecosystem ?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
i think its okay to allow this without version constraint
There was a problem hiding this comment.
♻️ Duplicate comments (1)
spec/integration/manifest.spec.yml (1)
298-310:⚠️ Potential issue | 🟠 MajorMissing backward-compat patch for
deployment_modes.agentless.release.Line 298 introduces a new field, but there is no
versionspatch removing it forbefore: 3.6.1. Without that, olderformat_versionschemas can incorrectly acceptpolicy_templates[].deployment_modes.agentless.release.Proposed fix
versions: + - before: 3.6.1 + patch: + - op: remove + path: "/definitions/deployment_modes/properties/agentless/properties/release" - before: 3.6.0 patch: # Support for otelcol and dynamic_signal_types in integration packages. - op: addAs per coding guidelines: "Version patches enable backward compatibility by removing features from older spec versions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/integration/manifest.spec.yml` around lines 298 - 310, Add a backward-compatibility version patch that removes the new field deployment_modes.agentless.release for older schema versions (before: 3.6.1); specifically create a patch under the manifest schema's versions array that targets before: 3.6.1 and deletes the policy_templates[*].deployment_modes.agentless.release entry so older format_version values will not accept this new field. Ensure the patch references the exact path policy_templates[].deployment_modes.agentless.release and follows existing patch structure used for other removed fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@spec/integration/manifest.spec.yml`:
- Around line 298-310: Add a backward-compatibility version patch that removes
the new field deployment_modes.agentless.release for older schema versions
(before: 3.6.1); specifically create a patch under the manifest schema's
versions array that targets before: 3.6.1 and deletes the
policy_templates[*].deployment_modes.agentless.release entry so older
format_version values will not accept this new field. Ensure the patch
references the exact path policy_templates[].deployment_modes.agentless.release
and follows existing patch structure used for other removed fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7851cc51-f971-4bee-83b9-859483422622
📒 Files selected for processing (1)
spec/integration/manifest.spec.yml
| // is the authoritative source of maturity and an explicit override would conflict. | ||
| if len(manifest.PolicyTemplates) == 1 { | ||
| tmpl := manifest.PolicyTemplates[0] | ||
| isSingleDeployment := tmpl.DeploymentModes.Default.Enabled != nil && !*tmpl.DeploymentModes.Default.Enabled |
There was a problem hiding this comment.
will it be single too if Default.Enabled == nil ?
maybe we can extract this to a smaller helper validateAgentlessReleaseDeployment so the main function is cleaner? we could just add unit tests based on the input (manifest struct), and then add test under the validation tests packages with fixtures, wdyt?
There was a problem hiding this comment.
Good idea! Added this in 789b7d3
will it be single too if Default.Enabled == nil ?
From my understanding if Default.Enabled == nil then absence get translated into implicitly supporting "agent" deployment, which means we have more than 1 deployment mode with agentless.
Co-authored-by: Tere <romero.teresa@protonmail.com>
|
@teresaromero Changed the direction here a little bit, to just accept |
💚 Build Succeeded
History
|
MichelLosier
changed the title
|
test integrations |
|
Created or updated PR in integrations repository to test this version. Check elastic/integrations#18315 |
3 participants
What does this PR do?
releasefield in the agentless deployment mode. This is to help indicate a different maturity level than that of the package semver. Accepts valuesbeta,preview.Why is it important?
Agentless deployment modes for an integration can be at a different level of functional maturity than the default deployment of an integration. This will help communicate that expectation to users downstream in Kibana.
Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues