Skip to content

[mage] Unify packaging targets#14871

Draft
swiatekm wants to merge 7 commits into
mainfrom
mage/unify-packaging
Draft

[mage] Unify packaging targets#14871
swiatekm wants to merge 7 commits into
mainfrom
mage/unify-packaging

Conversation

@swiatekm

@swiatekm swiatekm commented Jun 11, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Unifies the mage packaging targets: mage package and mage packageUsingDRA. As a result, the latter is folded into the former.

To achieve this, we introduce a new configuration variable: AGENT_CORE_SOURCE. The value can either be local or manifest and controls whether the agent core package is built locally or downloaded from the manifest.

We also set some defaults to make local work easier.

  • USE_PACKAGE_VERSION is now true by default. We build from the manifest in the .package-version file rather than take the latest one.
  • EXTERNAL is now true by default. Packaging using locally built dependencies in infrequent, especially after agentbeat was folded into the otel collector binary.

Why is it important?

Checklist

  • I have read and understood the pull request guidelines of this project.
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

Disruptive User Impact

How to test this PR locally

Related issues

@mergify

mergify Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This pull request does not have a backport label. Could you fix it @swiatekm? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-./d./d is the label that automatically backports to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@swiatekm swiatekm force-pushed the mage/unify-packaging branch from 446e929 to 5bcbfe2 Compare June 11, 2026 09:08
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

golangci-lint failed in lint (ubuntu-latest) at step golangci-lint due to a gosec G706 log-injection finding in dev-tools/mage/settings.go. Stop logging raw AGENT_CORE_SOURCE input (prefer returning an explicit validation error for unknown values), then re-run the workflow.

Remediation

  • In loadPackagingSettingsFromEnv(), replace the unknown AGENT_CORE_SOURCE warning path that logs raw env input with explicit validation handling (e.g., return an error for invalid values, or log a fixed message without interpolating user input).
  • Update/add tests in dev-tools/mage/settings_test.go to cover invalid AGENT_CORE_SOURCE behavior.
  • Re-run golangci-lint for PR #14871.
Investigation details

Root Cause

The lint run includes a tainted-input log statement for AGENT_CORE_SOURCE; gosec flags it as G706 (log injection via taint analysis).

Evidence

##[error]dev-tools/mage/settings.go:1580:13: G706: Log injection via taint analysis (gosec)
		log.Printf("Warning: unknown AGENT_CORE_SOURCE=%q; defaulting to %q", v, CoreSourceLocal)
		          ^
##[error]issues found

Validation

  • Not run locally; diagnosis is based on the failing job logs for run 27336278084.

Follow-up

If you keep non-fatal fallback behavior for unknown values, ensure logs do not include raw env-derived input.


What is this? | From workflow: PR Actions Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@github-actions

This comment has been minimized.

@swiatekm swiatekm changed the title Mage/unify packaging [mage] Unify packaging targets Jun 11, 2026
@swiatekm swiatekm added skip-changelog chore Tasks that just need to be done, they are neither bug, nor enhancements backport-active-all Automated backport with mergify to all the active branches labels Jun 11, 2026
@mergify

mergify Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b mage/unify-packaging upstream/mage/unify-packaging
git merge upstream/main
git push upstream mage/unify-packaging

swiatekm and others added 5 commits June 11, 2026 14:16
Remove stale/verbose comments, redundant inline remarks, and a spurious
double-space. Fix package.sh shebang to #!/usr/bin/env bash. Remove the
obvious FIPS-block comments whose meaning is already clear from the
conditional structure.

Also add §8 to packaging-analysis.md documenting the USE_PACKAGE_VERSION=true
default and the two callers that must opt out.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@swiatekm swiatekm force-pushed the mage/unify-packaging branch from 79a4ed2 to ddaa589 Compare June 11, 2026 12:17
@swiatekm swiatekm force-pushed the mage/unify-packaging branch from e739a87 to ecad7f3 Compare June 11, 2026 14:21
@elasticmachine

elasticmachine commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

All 4 failed Buildkite packaging jobs in build 41330 fail for the same deterministic config conflict: MANIFEST_URL is set while USE_PACKAGE_VERSION is still true when mage first loads settings. Move the MANIFEST_URL guard so USE_PACKAGE_VERSION=false is exported before the first mage command.

Remediation

  • In .buildkite/scripts/steps/package.sh, set AGENT_CORE_SOURCE=manifest and USE_PACKAGE_VERSION=false before the first mage invocation (mage clean).
  • Re-run elastic-agent-package build 13317 after that reorder.
Investigation details

Root Cause

dev-tools/mage/settings.go now enforces a hard conflict when both are active (loadPackagingSettingsFromEnv, L1549-L1567 in PR head):

aif manifestURLSet && s.Packaging.UsePackageVersion {
    return errors.New("MANIFEST_URL and USE_PACKAGE_VERSION=true are mutually exclusive ...")
}

But .buildkite/scripts/steps/package.sh currently runs mage clean first (~L8) and only later applies the guard (~L14-L17):

mage clean
...
if [ -n "${MANIFEST_URL:-}" ]; then
  export AGENT_CORE_SOURCE=manifest
  export USE_PACKAGE_VERSION=false
fi

So settings are loaded and fail before the override can take effect.

Evidence

  • Build: https://buildkite.com/elastic/elastic-agent/builds/41330
  • Failed jobs:
    • :package: FIPS=false Cross Building and package elastic-agent
    • :package: FIPS=true Cross Building and package elastic-agent
    • :package: FIPS=false Package ARM elastic-agent
    • :package: FIPS=true Package ARM elastic-agent
  • Key log excerpt (present in each failed log):
Error: failed to load settings: loading packaging settings: MANIFEST_URL and USE_PACKAGE_VERSION=true are mutually exclusive: USE_PACKAGE_VERSION reads .package-version to set the manifest URL; set USE_PACKAGE_VERSION=false when providing MANIFEST_URL explicitly

Verification

  • Reproduced via the 4 Buildkite job logs for build 41330.
  • Checked for matching flaky-test issue labels; none matched this failure signature.

Follow-up

If the guard reorder is intentional and mage clean should remain first, an alternative is to make mage clean not require loading packaging settings.


What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-active-all Automated backport with mergify to all the active branches chore Tasks that just need to be done, they are neither bug, nor enhancements skip-changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants