-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
/package_policies: package vars should be on input level #125625
Comments
Pinging @elastic/fleet (Team:Fleet) |
@juliaElastic @criamico you all worked on recent changes to our API in 8.0. Was this one of the breaking changes we made to the package policies API? Possibly as part of #119739? |
@mtojek Curious that we just found this failing test now. Do we know how long this test has been failing against the 8.0 branch? Maybe a recent change that hasn't been shipped is creating this problem |
I can explain this. We used to test AWS integration against the 7.x stack and now we're defaulting to 8.x. Recently we had a lot of broken tests in both lines, so it's hard to figure out when it started failing. We also don't keep artifacts for a longer time, but based on CI statuses, I think it worked around ~2021-12-22. |
I think this is a bug and not an intentional or unintentional breaking change because we still have |
Nevermind I found them, this definitely looks strange, see the
|
Seems related to #124215 |
@joshdover yes, we introduced stricter validation in that pr, meaning that the validation rejects package policies where there are unknown vars provided (missing from policy template) |
Hey @juliaElastic, could you please refer to the sample I posted above and adjust it to pass the validation? I will review it and see what we can do in elastic-package. BTW in this case it looks like an unintentional breaking change. EDIT: EDIT2: I think you might be talking about a different problem, as all vars are reported as missing, even ones that were passed (access key, secret key). |
@mtojek so the validation checks against the package definition from the api call below:
the problematic vars are on different level of the package definition, outside of inputs/streams. we debated whether this validation is breaking, and thought that it is an improvement not to allow invalid policies to be created: https://github.com/elastic/kibana/pull/124215/files#r796666631 cc @joshdover |
I think, global vars are intended and were introduced as part of works on input groups? The manifest is compatible with Kibana since 7.15. cc @kaiyan-sheng
The problem here is that we use this system test for a long time and it worked well :) Notice: if we decide to change its format, then we have to adjust the elastic-package tool. EDIT: I'm trying to understand why did our system test config work well with 7.x and it's invalid with 8.x? elastic-package passes only vars which are defined in manifests. I assume that the root cause is the wrong location? |
okay, if that's the case, it seems that Fleet validation should consider it valid.
yes, the location is the cause I think |
Thank @juliaElastic for taking care of this! If this is only about the system tests of AWS integration, we can temporarily disable it. It looks like our end-users are safe. If you plan to fix it in a few days, we're fine with that schedule :) |
I raised the pr with the revert, should be merged soon once reviewed. EDIT: the revert is merged |
one thing I'm not sure about, I tried creating that package_policy in the description (after disabling the validation locally), and on the UI the values of those vars are not loading. strangely I see only the Endpoint value there, which seems to be set on the outermost vars too |
Thank you for looking into this!!
Yes global vars are intended. Because these variables are shared between all data streams inside the AWS package. @juliaElastic |
Yes, it works if I put it in global vars (outside of inputs), but my question is, the same vars can appear inside inputs/streams as well, should Fleet take the values any level (e.g. moved inside inputs) and load them on UI? |
@juliaElastic 👍 Thanks for confirming the global vars work. For AWS package, the same vars will not appear inside inputs/streams. @mtojek Do you know if there's any other use case with variables need to appear inside and outside the inputs? |
AFAIK only the AWS package uses vars so extensively and maybe the GCP soon as @endorama is experimenting with system tests now. |
@juliaElastic @kaiyan-sheng I can confirm that this issue will also affect GCP: elastic/elastic-package#701 |
@mtojek the unknown vars validation has been reverted, we would have to check where the GCP error comes from exactly. can you give the package policy object and the new package definition in JSON format? |
It also depends on when the Kibana Docker image was published. I'm checking what's the status now and will come back with any feedback: pipeline job. Keep in mind that the GCP package is under development, so I can't guarantee that it's 100% correct. |
@juliaElastic Ok, I checked the status of AWS and GCP packages and both are failing with similar error: GCP:
AWS:
EDIT: I checked the CI job for Integrations. It uses stack 8.1.0-SNAPSHOT and it looks like the issue is fixed. The elastic-package tool uses 8.0.0 (RELEASE), so I bet it might be the root cause. |
.. and it was the root cause (confirmed with: elastic/elastic-package#708). The revert went well, so if there is any other action required we can close this issue. I do appreciate your help and ownership, @juliaElastic. Thank you! |
Thanks everyone for your effort on this issue! Given what I'm seeing here, is input level variables still missing from Fleet? The error was in the validation code, but is not clear to me if input level variables are correctly displayed in Fleet UI. |
Going to close this issue as it looks like everything has been resolved. |
Hi,
We're investigating an issue with AWS integration, system tests and policy vars. Everything works well with 7.17.0 branch, but when running system tests against 8.0, 8.1, Kibana fails with:
I checked the behavior using UI and it worked (modified a package policy), so I went down the rabbit hole and compared JSON requests:
elastic-package:
Kibana UI:
As you can see Kibana UI placed
vars
under the root. Is it correct behavior? We haven't touched that part of logic of elastic-package for a long time and it worked well. Something is definitely inconsistent between 7.x and 8.x.The text was updated successfully, but these errors were encountered: