Support source overrides for composable integration test packages#3474
Support source overrides for composable integration test packages#3474teresaromero merged 10 commits intoelastic:mainfrom
Conversation
- Introduced `RequiredInputsResolver` to manage local source overrides for input packages in the test runners. - Updated `testRunnerAssetCommandAction`, `testRunnerSystemCommandAction`, and `testRunnerPolicyCommandAction` to utilize the new resolver. - Modified `BuildPackage` to conditionally bundle input packages based on the presence of a resolver. - Added tests for `RequiresSourceOverrides` functionality to ensure correct resolution of local paths. - Refactored related structures and methods to support the new resolver options.
- Added `FleetPolicyPackageRoot` function to determine the appropriate package root for reading manifests in composable integrations. - Introduced tests for `FleetPolicyPackageRoot` to validate behavior with various package structures and requirements. - Updated `createPackagePolicy` and `createIntegrationPackagePolicy` functions to utilize the new package root resolution logic. - Refactored related code in `fleetpolicy.go` and `tester.go` to ensure compatibility with the new manifest handling approach.
…figuration - Updated `testRunnerAssetCommandAction`, `testRunnerSystemCommandAction`, and `testRunnerPolicyCommandAction` to use source overrides from the global test configuration. - Enhanced error handling for resolving source overrides. - Refactored `RequiresSourceOverrides` and `MergedRequiresSourceOverrides` methods to improve clarity and functionality. - Added tests for validating the new source override logic and handling of package requirements. - Introduced new configuration structure for package requirements in `globaltestconfig.go`.
- Added a new script `test-composable-packages.sh` to facilitate testing of composable packages. - Updated the Makefile to include a target for `test-check-packages-composable`. - Refactored paths in tests to align with the new directory structure under `test/packages/composable`. - Introduced two new composable packages: `01_ci_input_pkg` and `02_ci_composable_integration`, with corresponding manifests and configurations. - Updated README files to reflect the new package structure and testing procedures. - Adjusted various test configurations to ensure compatibility with the new composable package setup.
- Renamed the Makefile target from `test-check-packages-composable` to `test-build-install-packages-composable` for clarity. - Updated the Buildkite pipeline script to reflect the new target name for composable package integration tests. - Modified the `test-composable-packages.sh` script to streamline the creation of the composable profile and adjusted the configuration to point to the local package registry URL at the appropriate stage.
jsoriano
left a comment
There was a problem hiding this comment.
We can probably use built packages in all cases for system testing, this would likely simplify things, and would ensure better that we test what the user receives.
| func FleetPolicyPackageRoot(sourcePackageRoot string) (string, error) { | ||
| manifest, err := packages.ReadPackageManifestFromPackageRoot(sourcePackageRoot) | ||
| if err != nil { | ||
| return "", fmt.Errorf("read package manifest: %w", err) | ||
| } | ||
| if manifest.Type != "integration" { | ||
| return sourcePackageRoot, nil | ||
| } | ||
| if manifest.Requires == nil || len(manifest.Requires.Input) == 0 { | ||
| return sourcePackageRoot, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
It would be actually safer to use the built manifest in all cases, right? Even if it is the same as the source package for non-composable packages.
Then we could maybe directly use BuildPackagesDirectory instead of FleetPolicyPackageRoot, it already reads the package manifest.
There was a problem hiding this comment.
yes, agree. i did this change to narrow for composable but i think is safer to use the built version in any case. i spotted this when testing as type was not at the source package. i will change this
| builtManifest := filepath.Join(builtRoot, packages.PackageManifestFile) | ||
| if _, err := os.Stat(builtManifest); err != nil { | ||
| if os.IsNotExist(err) { | ||
| return "", fmt.Errorf("built package manifest not found at %s (build or install the package before creating Fleet policies for composable integrations): %w", builtManifest, err) | ||
| } | ||
| return "", fmt.Errorf("stat built package manifest %s: %w", builtManifest, err) | ||
| } |
There was a problem hiding this comment.
BuildPackagesDirectory already ensures that the built package has a manifest, it is required to get the name and version of the package.
| builtManifest := filepath.Join(builtRoot, packages.PackageManifestFile) | |
| if _, err := os.Stat(builtManifest); err != nil { | |
| if os.IsNotExist(err) { | |
| return "", fmt.Errorf("built package manifest not found at %s (build or install the package before creating Fleet policies for composable integrations): %w", builtManifest, err) | |
| } | |
| return "", fmt.Errorf("stat built package manifest %s: %w", builtManifest, err) | |
| } |
| if err != nil { | ||
| return "", fmt.Errorf("bundling input package templates failed: %w", err) | ||
| } | ||
| } |
|
|
||
| func createPackagePolicy(policy FleetAgentPolicy, packagePolicy FleetPackagePolicy) (*kibana.PackagePolicy, error) { | ||
| manifest, err := packages.ReadPackageManifestFromPackageRoot(packagePolicy.PackageRoot) | ||
| root, err := builder.FleetPolicyPackageRoot(packagePolicy.PackageRoot) |
There was a problem hiding this comment.
As mentioned in other comment, we might use the same code for all packages, though then we need to ensure that it is built first. Though this is probably fine.
| root, err := builder.FleetPolicyPackageRoot(packagePolicy.PackageRoot) | |
| root, err := builder.BuildPackagesDirectory(packagePolicy.PackageRoot, "") |
| // Match Fleet's view of the package: composable integrations use the built tree (requires.input). | ||
| root, err := builder.FleetPolicyPackageRoot(packageRoot) | ||
| if err != nil { | ||
| return kibana.PackagePolicy{}, "", "", err | ||
| } | ||
| usePkg := pkg | ||
| useDs := ds | ||
| useTempl := policyTemplate | ||
| if root != packageRoot { | ||
| // Composable integrations: input types are resolved on the built tree (requiredinputs.Bundle); | ||
| // reload manifests so Fleet policy keys match the package installed in setup (see FleetPackage). | ||
| pm, err := packages.ReadPackageManifestFromPackageRoot(root) |
There was a problem hiding this comment.
As mentioned in other comments, we can probably read the package manifest from the built package already in the caller, so we don't need to handle this here.
We need to build packages in any case for system tests, and for non-composable the built and source manifest will be the same.
Using the built manifest is safer in any case, we might introduce in the future other features that modify it in build time.
| if [ "${packageTestType}" == "false_positives" ]; then | ||
| continue | ||
| fi | ||
| # Composable packages require the local registry to be running during build; |
There was a problem hiding this comment.
When using source the built packages could be retrieved from the local build directory.
Is this required for packages referenced by name/version?
There was a problem hiding this comment.
no, if the test does not configure override or the override is a package it should download from the registry. i am reviewing this case as its not covered
There was a problem hiding this comment.
the override is only applicable at package tests. when running these test of build and install command there is no override and the registry is required. in this case local.
There was a problem hiding this comment.
this is a guard to keep current build and install tests as is and have a dedicated pipeline for composable cases
- Deleted the FleetPolicyPackageRoot function from builder package, which was responsible for determining the package root for reading manifests in composable integrations. - Removed associated test file fleet_policy_package_root_test.go that contained tests for the FleetPolicyPackageRoot function. - Updated references in fleetpolicy.go and tester.go to utilize BuildPackagesDirectory instead of FleetPolicyPackageRoot for resolving package roots. - Adjusted error handling and manifest reading logic to ensure compatibility with the new approach.
- Introduced `ci_composable_no_override` package with a manifest that requires `ci_input_pkg` without a source override. - Created configuration files for testing, including parallel execution settings. - Added data stream definitions for `ci_composable_logs`, including input variables and paths for log collection. - Developed expected policy outputs and test policies to validate the integration. - Updated README documentation to guide users on building and testing the new package.
- Added detailed instructions for testing composable packages with source overrides in `dependency_management.md`. - Updated `policy_testing.md` to include the `requires` key for specifying local package paths or registry versions. - Introduced new test fixtures under `internal/requiredinputs/testdata/` to demonstrate various scenarios of variable merging and template path handling. - Removed outdated manual package configurations and documentation to streamline the testing process. - Improved clarity in test cases and added README files for new test fixtures to guide users on expected behaviors.
mrodm
left a comment
There was a problem hiding this comment.
LGTM!
Added a couple of questions/suggestions related to the CI scripts.
Remove redundant stack shellinit from composable build/install script; Kibana and registry clients use the active profile after stack up. Move the composable integration test step next to other build-install-zip steps in the Buildkite pipeline generator for consistency. Made-with: Cursor
💚 Build Succeeded
History
|
|
@mrodm after addressing your feedback i am mergin this. if any comment on the changes for the review i can address them on the next followup |
Summary
Closes #3464.
Implements local
sourceresolution forrequires.inputdependencies so a composable integration and its required input packages can be developed and tested in the same repository without publishing to EPR first.This is the follow-up work tracked from the review of #3459.
Changes
globaltestconfig.goto parse and exposesourceoverrides from_dev/test/config.ymlpackage requirements.asset,system, andpolicytest runners to resolvesourceoverrides from the global test configuration instead of downloading from EPR.FleetPolicyPackageRoot: New function ininternal/builderto determine the correct package root when resolving manifests for composable integrations.requiredinputs: RefactoredRequiresSourceOverridesandMergedRequiresSourceOverridesto support local path resolution per entry.test-composable-packages.shscript andtest-check-packages-composableMakefile target; wired into the Buildkite integration test pipeline.01_ci_input_pkgand02_ci_composable_integrationpackages undertest/packages/composable/to exercise the new path without hitting EPR.Co-authored-by: Cursor and Claude