Add support for release field in agentless deployment mode#1130
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)
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. 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
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.
|
@teresaromero Changed the direction here a little bit, to just accept |
MichelLosier
changed the title
|
test integrations |
|
Created or updated PR in integrations repository to test this version. Check elastic/integrations#18315 |
| - description: Add optional `release` field to agentless deployment mode to explicitly declare its release stage. | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/1130 |
There was a problem hiding this comment.
This should be moved to a new section 3.6.3-next, since 3.6.0-3.6.2 were already released/published.
| enum: | ||
| - beta | ||
| - ga |
There was a problem hiding this comment.
At the moment of reviewing this PR, there are 70 packages using deployment_modes in their manifests in the integrations repository.
That would mean that all of them would be considered as beta when the kibana change is merged (and released). Could that be an issue ?
There was a problem hiding this comment.
Ya that is currently the intent, all agentless deployments are considered beta except for CSPM.
That said, currently re-visiting the default value behavior here: #1098 (comment)
I've staged up a PR for CSPM as GA: elastic/integrations#18552
Co-authored-by: Tere <romero.teresa@protonmail.com>
MichelLosier
force-pushed
the
1098-add-agentless-deployment-release-field
branch
from
041333f to
9c48de5
Compare
teresaromero
left a comment
teresaromero
left a comment
There was a problem hiding this comment.
👍🏻 . i wonder if this is not a patch but a minor version update, as we are adding a new field 🤔
Good catch, it does require a Kibana version to be interpreted, but that said it does not require packages to constrain to that Kibana version. Docs will consume this separately. The next minor does seem to be the better home. I've also updated the description of the field regarding the defaulting behavior in Kibana as discussed in the most recent comments here: #1098 This should be all the changes intended from my end. |
💚 Build Succeeded
History
|
5 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,ga. Absence impliesbetaby defaultWhy 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