[Integration Test] Ensure that upgrading a FIPS-capable Agent results in a FIPS-capable Agent #7804
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
|
ycombinator
added
Team:Elastic-Agent-Control-Plane
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
ycombinator
commented
Apr 9, 2025
ycombinator
force-pushed
the
it-upgrade-fips-to-fips
branch
from
d71c0a5 to
1bf616f
Compare
michel-laterman
previously approved these changes
Apr 17, 2025
ycombinator
force-pushed
the
it-upgrade-fips-to-fips
branch
from
917b25c to
2ced379
Compare
ycombinator
added
backport-8.19
ycombinator
changed the title
ycombinator
force-pushed
the
it-upgrade-fips-to-fips
branch
from
2ced379 to
da48fd6
Compare
|
It turns out we currently have no way of upgrade a FIPS-capable Agent to another version of a FIPS-capable Agent. I just tried this manually and it doesn't work: Moving this PR into draft while I implement the missing capability. |
|
The @michel-laterman @simitt I think we need https://github.com/elastic/ingest-dev/issues/5264 for these tests? |
Agree |
ycombinator
force-pushed
the
it-upgrade-fips-to-fips
branch
from
2e9cd6a to
531c3f3
Compare
|
CI is failing only on the new test introduced in this PR, Definitely looks like a FIPS-related error. Investigating... |
ycombinator
dismissed stale reviews from cmacknz and michel-laterman
via
d770d77
ycombinator
commented
Jun 11, 2025
blakerouse
previously approved these changes
Jun 12, 2025
cmacknz
previously approved these changes
Jun 12, 2025
|
💛 Build succeeded, but was flaky
Failed CI StepsHistory
cc @ycombinator |
cmacknz
approved these changes
Jun 13, 2025
mergify bot
pushed a commit
that referenced
this pull request
Jun 13, 2025
… in a FIPS-capable Agent (#7804) * Adding skeleton for FIPS-to-FIPS upgrade test * Expose FIPS compliance in GRPC client Version call response * Test upgrade from FIPS to FIPS artifact * Change assert to require * Add postWatcherSuccessHook to upgrade test * Refactor standalone upgrade test to take upgradeOpts * Fix up FIPS upgrade test to use postWatcherSuccessHook to test FIPS compliance of upgraded Agent * Add version constraint to test * s/compliant/capable/ * s/compliant/capable/ * Append -fips to artifact name if current release of Agent is FIPS-capable * Enable FIPS-capable to FIPS-capable Agent upgrades * Running mage fmt * Adding test to ensure FIPS-capable Agent cannot be upgraded to non-FIPS-capable Agent * Adding return value * Fixing function calls * Remove post-upgrade success hook since we expect upgrade to fail * Add minimum FIPS version check for start version * Simplify upgradeOpts initialization * Add version equality comparison method * Fix version checks in tests * Refactor version check into own helper function * Fixing args * No need to pass testing.T * Remove redundant test case * Restrict FIPS integration tests to Linux * Add Fleet-managed Agent FIPS upgrade test * Remove integration test trying to upgrade FIPS to non-FIPS * Fix type of argument * Refactoring: extract common logic into helper function * Remove old code * Require no error * Fixing syntax errors * Define tests as needing a FIPS environment * FIPS upgrade tests should only run on Linux * FIPS upgrade tests should start with FIPS-capable version * Fixing comment + skip message * Fix syntax errors * Removing TestStandaloneUpgradeFIPStoFIPS test * Removing TestFleetManagedUpgradePrivilegedFIPS test * Add back accidentally-deleted function * Combine less and equal unit tests * Hash replaceToken only if its non-empty * Use startFixture (cherry picked from commit ac9ee9a) # Conflicts: # internal/pkg/agent/application/upgrade/upgrade.go # testing/upgradetest/upgrader.go
8 tasks
simitt
reviewed
Jun 16, 2025
| "Minimum start version of FIPS-capable Agent for running this test is either %q or %q, current start version: %q", | ||
| *upgradetest.Version_8_19_0_SNAPSHOT, | ||
| *upgradetest.Version_9_1_0_SNAPSHOT, | ||
| currentVersion, |
There was a problem hiding this comment.
I wonder if we should rather only backport this to 8.19 and 9.1? The current behaviour has the potentialof the test being skipped for unwanted reasons without anyone noticing.
There was a problem hiding this comment.
I'm not sure I'm following because this PR is indeed only merged into main (so 9.1) and backported to 8.19.
There was a problem hiding this comment.
In that case, why is this isFIPSCapableVersion check necessary and skipping tests otherwise?
There was a problem hiding this comment.
Good catch — I think this test needs to be rewritten a bit!
Some of our upgrade tests loop over the versions listed in https://github.com/elastic/elastic-agent/blob/main/testing/integration/testdata/.upgrade-test-agent-versions.yml and, for each version in that list, start the upgrade from that version of Agent and upgrade the Agent to whatever version is defined by the branch on which the test is running. I was thinking this test would do the same and, therefore, added this check to make sure we only consider versions for which we have FIPS-capable artifacts.
However, reading through the test code, it's not actually considering the version list at all. I think it should, though; I will rewrite the test to do that as part of the unified test I'm working on in #8383.
| operatingSystem string | ||
| arch string | ||
| expectedName string | ||
| expectedErr string |
There was a problem hiding this comment.
Nit: not used anymore in these tests, also version, operatingSystem and arch do not differ in the below testcases, so might be able to tidy this up a bit.
ycombinator
added a commit
that referenced
this pull request
Jun 17, 2025
… in a FIPS-capable Agent (#7804) * Adding skeleton for FIPS-to-FIPS upgrade test * Expose FIPS compliance in GRPC client Version call response * Test upgrade from FIPS to FIPS artifact * Change assert to require * Add postWatcherSuccessHook to upgrade test * Refactor standalone upgrade test to take upgradeOpts * Fix up FIPS upgrade test to use postWatcherSuccessHook to test FIPS compliance of upgraded Agent * Add version constraint to test * s/compliant/capable/ * s/compliant/capable/ * Append -fips to artifact name if current release of Agent is FIPS-capable * Enable FIPS-capable to FIPS-capable Agent upgrades * Running mage fmt * Adding test to ensure FIPS-capable Agent cannot be upgraded to non-FIPS-capable Agent * Adding return value * Fixing function calls * Remove post-upgrade success hook since we expect upgrade to fail * Add minimum FIPS version check for start version * Simplify upgradeOpts initialization * Add version equality comparison method * Fix version checks in tests * Refactor version check into own helper function * Fixing args * No need to pass testing.T * Remove redundant test case * Restrict FIPS integration tests to Linux * Add Fleet-managed Agent FIPS upgrade test * Remove integration test trying to upgrade FIPS to non-FIPS * Fix type of argument * Refactoring: extract common logic into helper function * Remove old code * Require no error * Fixing syntax errors * Define tests as needing a FIPS environment * FIPS upgrade tests should only run on Linux * FIPS upgrade tests should start with FIPS-capable version * Fixing comment + skip message * Fix syntax errors * Removing TestStandaloneUpgradeFIPStoFIPS test * Removing TestFleetManagedUpgradePrivilegedFIPS test * Add back accidentally-deleted function * Combine less and equal unit tests * Hash replaceToken only if its non-empty * Use startFixture (cherry picked from commit ac9ee9a)
ycombinator
added a commit
that referenced
this pull request
Jun 23, 2025
…PS-capable Agent results in a FIPS-capable Agent (#8491) * [Integration Test] Ensure that upgrading a FIPS-capable Agent results in a FIPS-capable Agent (#7804) * Adding skeleton for FIPS-to-FIPS upgrade test * Expose FIPS compliance in GRPC client Version call response * Test upgrade from FIPS to FIPS artifact * Change assert to require * Add postWatcherSuccessHook to upgrade test * Refactor standalone upgrade test to take upgradeOpts * Fix up FIPS upgrade test to use postWatcherSuccessHook to test FIPS compliance of upgraded Agent * Add version constraint to test * s/compliant/capable/ * s/compliant/capable/ * Append -fips to artifact name if current release of Agent is FIPS-capable * Enable FIPS-capable to FIPS-capable Agent upgrades * Running mage fmt * Adding test to ensure FIPS-capable Agent cannot be upgraded to non-FIPS-capable Agent * Adding return value * Fixing function calls * Remove post-upgrade success hook since we expect upgrade to fail * Add minimum FIPS version check for start version * Simplify upgradeOpts initialization * Add version equality comparison method * Fix version checks in tests * Refactor version check into own helper function * Fixing args * No need to pass testing.T * Remove redundant test case * Restrict FIPS integration tests to Linux * Add Fleet-managed Agent FIPS upgrade test * Remove integration test trying to upgrade FIPS to non-FIPS * Fix type of argument * Refactoring: extract common logic into helper function * Remove old code * Require no error * Fixing syntax errors * Define tests as needing a FIPS environment * FIPS upgrade tests should only run on Linux * FIPS upgrade tests should start with FIPS-capable version * Fixing comment + skip message * Fix syntax errors * Removing TestStandaloneUpgradeFIPStoFIPS test * Removing TestFleetManagedUpgradePrivilegedFIPS test * Add back accidentally-deleted function * Combine less and equal unit tests * Hash replaceToken only if its non-empty * Use startFixture (cherry picked from commit ac9ee9a) * Fixing conflicts --------- Co-authored-by: Shaunak Kashyap <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR allows a FIPS-capable Agent to upgrade to another FIPS-capable Agent. It also adds an integration test,
TestFleetManagedUpgradeUnprivilegedFIPS, to check that a Fleet-managed FIPS-capable unprivileged Agent will upgrade only to another FIPS-capable Agent.Why is it important?
To preserve FIPS-compliance across upgrades.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesI have added tests that prove my fix is effective or that my feature worksI have added an entry in./changelog/fragmentsusing the changelog toolDisruptive User Impact
None; this PR adds an integration test.