-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add assistance to detect common backwards compatibilty issues #579
Comments
The idea is nice of course, but I don't see an easy way to evaluate these changes. A package is considered to be incompatible if we can't install it or tests are failing, so we could just remodel the testing pipeline and run tests for more stacks - this is the dumb way. To implement the smartest one, we need to collect some ideas. |
These checks wouldn't be related to multiple versions of the stack, but to multiple versions of the package itself. The E2E way to do it would be to install an old version of the package, upgrade to the new one, and do something as running the system tests, and check that everything works. But I think that many things can be "statically" detected. For example:
I think that this kind of issues are actually detected more reliably with static checks, other kind of tests as E2E or system tests may not use all fields, or variables, and then they wouldn't detect issues with them. For example incompatible changes on types can be easy to detect by comparing old and new fields definitions, these issues may not cause any issue during ingestion, but can cause problems in Kibana. Other breaking changes as the ones in policy templates may be more complicated, or not possible to detect without system tests, but we can incrementally add checks later if we find ways to implement them. |
The transform upgrade process that @qn895 is working on would also benefit from static checks against the previous package version. Since package upgrades cannot delete indices we're making a rule that if you change a transform in any way then you have to give it a different destination index, as it's too hard to be sure if the new transform and old destination index would be compatible. So ideally we'd validate that this rule has been followed rather than risking problems being found a while after install of the new package. |
@jsoriano @mrodm Shall we also piggy back on this for packages upgrade? We have seen failures where a package type was changed to an input package type and it broke the package upgrade. Relying on CI to test packages installation AND upgrade from the previous patch/minor would definitely be helpful. |
Changing the package type from integration to input should not be breaking, but yes, cases like this one should be detected at some testing stage. |
Hello, I have some questions:
|
I don't think so. Actually integrations for some propietary sources are usually tested with mocked data. Package developers need to explicitly check if formats have changed. There is an open issue focused on finding changes in [non-SaaS] services: #1902. In any case I would consider breaking changes in monitored services a separate issue. Here we intend to prevent the introduction of breaking changes in the package itself.
Not for changes in packages, this is the kind of check that we would introduce by solving this task. For changes in the stack that might produce issues with packages, we have https://buildkite.com/elastic/integrations-schedule-daily/, where we test against latest versions of 7.x, 8.x and serverless, though we don't test upgrades. Some changes in Fleet might be tested in Kibana CI itself, we have probably already some tests around there. |
It could be nice to have some feature to assist detecting backwards compatibility issues in packages. This could be part of
elastic-package check
, but it would have the particularity of needing to download (or checkout) previous versions of the package, so maybe it should be optional.There are some things that could be automatically checked between versions:
Proposed solution
We will add a new
elastic-package test compatibility
subcommand. In general this command will be used to compare the compatibility of the definitions in two versions of a package. This subcommand would run tests decided by flags or the environment. It would have different modes:[--from-git <git repository>] [--from-git-ref <git ref>]
Default execution would check if the current working copy is a git repository, with a branch forked from an upstream branch. If it is, it would checkout the package from the base commit to a temporary location and would compare with the local package. If it cannot obtain another package from the local repository, because it is not a repository or it doesn't contain a previous version of the package, the test does nothing. This could be extended with flags to select a source repository and commit.--from-registry <registry url> [--kibana-version <version>] [--all]
would compare the local package with versions published in the indicated registry. By default it would compare with the latest version published, but could be extended to support comparing with the package of a given kibana version, or with all packages.--from-registry <registry url> --package <package> --from-registry-version <version> --to-registry-version <version>
would compare package versions from the registry. It does not require a local copy. This could be used to check compatibility between versions already published.For any of the modes, the compatibility checks would be performed in the following ways:
ecs@mappings
and so on. Comparisons would be done by looping over all definitions in the old package, one by one, and checking for the presence of a compatible definition in the new one. We would also need to look for removed definitions. This would be the default.--system
System checks based on installing both package versions, dumping the generated assets, and comparing them following similar rules as the static checks. It would also do a dummy upgrade testing by installing one version after the other. The stack used would be the same as in other tests, the one in the current profile, or indicated by environment variables.The specific checks that will be performed on each execution would be the ones mentioned in the top of the description. We will start by implementing the tests for field mappings compatibility, and would follow by implementing the ones about variables.
Packages will have the option to skip these tests following the common settings we have for that in other tests. Additionally, breaking change notices in the changelogs can be used to circunvent detected failures.
We would start implementing the local git mode, with static checks, what could be already useful to detect many issues in fields changes. Other modes and tests would be implemented on following phases.
The text was updated successfully, but these errors were encountered: