Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose policy_templates_behavior and deployment modes for policy templates in search and package endpoints #1244

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Oct 28, 2024

Closes #1238
Closes #1243

Expose policy_templates_behavior and the deployment_modes for the policy templates (if any) in both search and package endpoints.

How to test

  1. Compile and run Elastic Package Registry with the test packages (using config.yml from the repo):
    eval "$(gvm $(cat .go-version))"
    go run . -config config.yml
  2. In another terminal, run these queries and ensure those new fields are present:
    curl -s "http://127.0.0.1:8080/search?package=deployment_modes&prerelease=true"
    curl -s "http://127.0.0.1:8080/package/deployment_modes/0.0.1/"

@mrodm mrodm self-assigned this Oct 28, 2024
@@ -184,6 +187,15 @@ type Download struct {
Type string `config:"type" json:"type" validate:"required"`
}

type DeploymentModes struct {
Default *DeploymentMode `config:"default,omitempty" json:"default,omitempty" yaml:"default,omitempty"`
Agentless *DeploymentMode `config:"agentless,omitempty" json:"agentless,omitempty" yaml:"agentless,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not added the organization, division and team fields that can also be set under agentless key:

https://github.com/elastic/package-spec/blob/cc5282374a5d062497bf0517d77a449d7176a35e/spec/integration/manifest.spec.yml#L177-L197

Should they be added? Are they necessary for Fleet?

Copy link
Member

Choose a reason for hiding this comment

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

According to the description these fields are only intended to be used internally, when the package is installed: elastic/package-spec#795
So I don't think we need to expose them here. cc @seanrathier just in case.

Copy link

@seanrathier seanrathier Oct 30, 2024

Choose a reason for hiding this comment

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

They need to be added because Kibana will send those fields to the Agentless API and then the Agentless API will tag the agentless agent deployment with that information.

Those fields should only be required if you have a deployment_modes agentless

Does that answer the question?

Copy link
Contributor Author

@mrodm mrodm Oct 30, 2024

Choose a reason for hiding this comment

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

Looking at this comment, I thought those values would be read directly from the manifest in Kibana (Fleet):
elastic/package-spec#795 (comment)

If the package is installed before sending those values to the Agentless API, could those values be read from the package manifest? If that is possible, probably it's not needed to add those fields to the API.

WDYT ? @seanrathier @jsoriano

Choose a reason for hiding this comment

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

If I am misunderstanding this request please let me know.

Are we looking to expose deployment_modes when searching for packages? If so, we don't need to expose the org, division and team in the search.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we looking to expose deployment_modes when searching for packages? If so, we don't need to expose the org, division and team in the search.

Exactly, I think I didn't give the full context here, sorry. This question was more related to check whether or not those fields should be added in to the search and package API responses from the package registry.

So, I think it's good to proceed as it is the PR now (just exposing if the mode is enabled or not).

Thanks @seanrathier !

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm marked this pull request as ready for review October 28, 2024 17:40
@mrodm mrodm requested a review from a team October 28, 2024 17:40
@@ -184,6 +187,15 @@ type Download struct {
Type string `config:"type" json:"type" validate:"required"`
}

type DeploymentModes struct {
Default *DeploymentMode `config:"default,omitempty" json:"default,omitempty" yaml:"default,omitempty"`
Agentless *DeploymentMode `config:"agentless,omitempty" json:"agentless,omitempty" yaml:"agentless,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

According to the description these fields are only intended to be used internally, when the package is installed: elastic/package-spec#795
So I don't think we need to expose them here. cc @seanrathier just in case.

@mrodm mrodm merged commit 5692478 into elastic:main Oct 30, 2024
5 checks passed
@mrodm mrodm deleted the expose-deployment-modes branch October 30, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants