Support building integration packages with required input dependencies#3459
Support building integration packages with required input dependencies#3459teresaromero wants to merge 28 commits intoelastic:mainfrom
Conversation
Support requires.input (and related manifest fields) for composable integration packages per package-spec. Made-with: Cursor
Download integration zip artifacts from EPR for required input resolution during build. Made-with: Cursor
Verify detached signatures when enabled via environment configuration. Made-with: Cursor
Download required input packages, copy policy and data stream agent templates (Fleet merge order), merge manifest variables, bundle data stream field definitions, and resolve package: stream references to concrete input types. Made-with: Cursor
Use profile-aware registry URLs for install, test, benchmark, and script runners; inject RequiredInputsResolver into the build pipeline. Made-with: Cursor
Fixtures cover template bundling, variable merge scenarios, field bundling, linked template_path, and stream resolution. Made-with: Cursor
Document requires.input build behavior and how to use a local or custom EPR during development. Made-with: Cursor
Handle closeFn errors in defer, trim always-nil error returns from merge helpers, add gocognit nolint for mergeVariables, and fix test assignments. Made-with: Cursor
Package archetypes have no required input dependencies, so BuildOptions.RequiredInputsResolver can be left nil. The builder already falls back to NoopRequiredInputsResolver when the field is unset, making the mock redundant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract per-manifest logic from bundleDataStreamTemplates into a new unexported processDataStreamManifest method, and add unit tests covering read failure, invalid YAML, unknown package (no write-back guard), partial stream errors, and no-package stream skipping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Files helper Extract the duplicated copy loop from collectAndCopyInputPkgPolicyTemplates and collectAndCopyInputPkgDataStreams into a single collectAndCopyPolicyTemplateFiles function parameterised by destDir. Remove the RequiredInputsResolver receiver from both wrappers since they don't use resolver state. Add unit tests for the new helper covering single/multiple template paths, deduplication, missing files, invalid paths, custom destDir, and content preservation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review Comments AddressedThis PR consolidates the work from #3329 and addresses all review comments. Summary below. @jsorianoFile naming — no underscores in Go filenames Rename Rename Reuse core logic from Variable naming matching YAML key YAML format preservation vs. struct approach Use Zip reader as filesystem instead of extracting Content packages in dependency docs Higher-level abstraction in Local cache for packages — deferred to a follow-up as agreed. Converge local registry serving methods — noted; left as a longer-term architectural refactor. Methods with @mrodmEPR URL should account for profile settings Docker image reference in docs Port conflict warning
Linked files / symlinks in policy templates Package signature validation Known follow-ups (not in scope for this PR)These were flagged by @jsoriano during testing and agreed to be addressed separately:
|
…277-composable-input-bundling
…277-composable-input-bundling
- Add test/packages/composable/01_ci_input_pkg and 02_ci_composable_integration for requires.input coverage (vars, fields, templates, mixed streams) - Phase-2 build in test-build-install-zip.sh after stack up; set package_registry.base_url to local EPR with restore in cleanup - Mirror composable skip + phase-2 in test-build-install-zip-file.sh - Point internal/requiredinputs integration tests at composable fixtures - Remove duplicate manual_packages; edge fixtures use ci_input_pkg; refresh manual_packages README Made-with: Cursor
Complete policy and data stream variable metadata (type, titles, flags) for package-spec validation. Use logfile for the native logs stream so Kibana accepts the data stream manifest. Update requiredinputs tests and the manual required_inputs fixture to match. Made-with: Cursor
test-build-zip runs after test-build-install-zip without a local registry; building 02_ci_composable_integration would download ci_input_pkg from production EPR and fail with 404. Mirror the install-zip phase-1 skip. Made-with: Cursor
| func PackageSignatureVerificationFromEnv() (verify bool, publicKeyPath string, err error) { | ||
| raw := os.Getenv(verifyPackageSignatureEnv) | ||
| if raw == "" { | ||
| return false, "", nil | ||
| } |
There was a problem hiding this comment.
As this process is going to be used to build the packages (downloading other packages from EPR) and publish those into package-registry (https://buildkite.com/elastic/integrations-publish), would it be possible to be enabled by default this verification ? So, the env variable could be like ELASTIC_PACKAGE_DISABLE_VERIFY_PACKAGE_SIGNATURE.
If so, probably it would be needed to ensure that the integrations repository (and elastic-package repo?) has its own copy of the public gpg key in a known location?
Or maybe, bundle the elastic public gpg key into elastic-package but allow to use other via an environment variable.
WDYT @teresaromero @jsoriano ?
There was a problem hiding this comment.
it would be needed to ensure that the integrations repository (and elastic-package repo?) has its own copy of the public gpg key in a known location
after investigating the integrations repo:
integrations repository does not store any gpg key. this is delegated to a custom service that is centralized in buildkite. when the package is built it is sent to the signing service and then the artifact is stored with the .sig file.
Or maybe, bundle the elastic public gpg key into elastic-package
I think that storing the key "hard-copy" at the repo can cause some trouble, when for example, the key is rotated.
overall, I think this process is not trivial, as we are injecting the download+verification dependency into the build process.
Security-wise, would this be a problem to leave the download part without signature verification?
I mean this to address this on a follow-up issue and keep the download package step without the verification. If ok i would revert the verification code and move it to a follow-up where we can discuss a better aproach cc @mrodm @jsoriano
There was a problem hiding this comment.
Agree with moving verification out of this PR. We can merge the basic functionality without this.
| return "", fmt.Errorf("opening downloaded package zip %s: %w", zipPath, err) | ||
| } | ||
| verifyErr := files.VerifyDetachedPGP(zipFile, sigBody, pubKey) | ||
| closeErr := zipFile.Close() |
There was a problem hiding this comment.
Why not using defer zipFile.Close() after opening the zip file
zipFile, err := os.Open(zipPath)
if err != nil {
_ = os.Remove(zipPath)
return "", fmt.Errorf("opening downloaded package zip %s: %w", zipPath, err)
}
defer zipFile.Close()
| for _, opt := range opts { | ||
| opt(c) | ||
| } | ||
| c.httpClient, _ = c.newHTTPClient() |
There was a problem hiding this comment.
Error here is silently ignored, it should be propagated. If we think that errors in certs.SystemPoolWithCACertificate can be ignored, please ignore the error explicitly when calling the method, and add an explaining comment.
|
|
||
| // mappingValue returns the value node for the given key in a YAML mapping node, | ||
| // or nil if the key is not present. | ||
| func mappingValue(node *yaml.Node, key string) *yaml.Node { |
There was a problem hiding this comment.
We have internal/yamledit, that has methods for getting, setting, deleting and printing yaml nodes. They could likely be reused here.
| // the top level) and a close function that must be called when done. For | ||
| // directory packages it closes the os.Root; for zip packages it closes the | ||
| // underlying zip.ReadCloser. | ||
| func openPackageFS(pkgPath string) (fs.FS, func() error, error) { |
There was a problem hiding this comment.
We might support only packages in zip format.
By now we are only supporting packages from the registry, that will come in this format. If we support local packages we can build them too.
NewClient now returns (*Client, error) so TLS/CA setup failures from newHTTPClient are visible to callers. DownloadPackage defers zip removal on failure after write, extracts verifyPackage for detached PGP checks, and defers closing the zip file during verification. Update all registry.NewClient call sites. Add revisionsFromRegistry in script tests to keep Run under gocyclo limits. Add tests for invalid CA paths/PEM, non-OK download responses, write failures, TLSSkipVerify construction, and revisionsFromRegistry behavior. Made-with: Cursor
Use stack.PackageRegistryBaseURL after loading the profile so elastic.epr.url matches install, test, and benchmark. Made-with: Cursor
Extract mergeVariables and resolveStreamInputTypes into focused helpers, rename promoted var scope types, and warn when input packages define multiple policy templates. Use errors.Is(os.ErrNotExist) in field bundling tests. Add unit tests for promoted override scoping and stream input resolution with multi-template input packages. Document new helpers and updated entry points. Made-with: Cursor
Buildkite runs test-build-install-zip-file.sh which uses yq to override package_registry.base_url for the composable phase-2 build. Include test-build-install-zip-file and test-build-install-zip-file-shellinit in the same with_yq branch as test-build-install-zip. Made-with: Cursor
Run elastic-package stack shellinit only when -s is used; apply manual exports only otherwise so the non-shellinit path is not polluted by shellinit exports. Made-with: Cursor
Made-with: Cursor
- Introduced new targets for composable integration tests in the Makefile. - Updated the Buildkite pipeline to include jobs for the new composable test targets. - Enhanced the integration test script to handle composable-only builds. - Modified the test-build-install-zip-file.sh script to support composable input packages. These changes facilitate dedicated testing for composable packages, improving CI coverage and ensuring proper integration.
Removed Windows-specific skip condition and adjusted directory handling in TestDownloadPackage_writeFailureCleansUp. The test now uses a temporary directory for zip file creation, ensuring proper cleanup after write failures. This enhances test reliability across platforms.
| if err != nil { | ||
| return fmt.Errorf("could not load profile: %w", err) | ||
| } | ||
| baseURL := stack.PackageRegistryBaseURL(prof, appConfig) |
There was a problem hiding this comment.
@jsoriano with the use of profile at the build command, we are coupling the stack with build. the command itself does not have the flag to select profile.
There was a problem hiding this comment.
We need it to support the local package registry, right?
What it is true is that it should remain working even if there is no stack started, this should be optional.
Replace all gopkg.in/yaml.v3 usage in internal/requiredinputs with github.com/goccy/go-yaml, aligning the package with internal/yamledit. Rewrite yamlutil.go to operate on goccy/go-yaml AST types and reuse yamledit.NewDocumentBytes for parsing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deleted the package signature verification code from the project, including related tests and environment variable documentation. This simplifies the codebase and removes unused functionality related to verifying downloaded packages from the Package Registry.
|
/test |
💔 Build Failed
Failed CI Steps
History
|
4 participants
Summary
Composable (integration) packages can declare
requires.inputso that, at build time, elastic-package downloads those input packages from the Elastic Package Registry (or the configuredpackage_registry.base_url), then updates the built package: agent templates are bundled with Fleet-correcttemplate_pathsorder, variables are merged into the integration and data stream manifests, field definitions are merged into data streamfields/, andpackage:references on policy inputs and data stream streams are replaced with the concrete input type from the resolved input package. This aligns with the composable packages direction in package-spec#1083.What ships (sub-issues / phases)
template_paths(input templates first, integration last).package:with resolvedtype:/input:so the built output matches hand-authored packages.Implementation
internal/requiredinputs/—RequiredInputsResolverimplementsBundle(buildPackageRoot)(nil resolver is a no-op). Pipeline: templates → variables → fields → stream resolution.internal/registry/—DownloadPackage, TLS options, tests.internal/files/— optional detached-signature verification (env-configured, off by default).internal/packages/— manifest types forrequires.input.internal/builder/packages.go— wiresRequiredInputsResolverinto the build.How to test
go test ./... make build format lint licenser gomod update