Skip to content

Conversation

smagnani96
Copy link
Contributor

@smagnani96 smagnani96 commented Aug 1, 2025

Commit 8dc7793 ("workflows/ipsec: Skip bpftrace DNS check if IPv4 is disabled") fixed the leak detection when running on an IPv6-only cluster. We forgot to disable bpftrace DNS checks for IPv6-only clusters in the "after downgrade" step. This commit should fix it.

Discussion #40412 (comment).

Fixes: #40868

Commit 8dc7793 ("workflows/ipsec: Skip bpftrace DNS check if IPv4
is disabled") fixed the leak detection when running on an IPv6-only
cluster. We forgot to disable bpftrace DNS checks for IPv6-only clusters
in the "after downgrade" step. This commit should fix it.

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96 smagnani96 self-assigned this Aug 1, 2025
@smagnani96 smagnani96 added release-note/ci This PR makes changes to the CI. feature/ipsec Relates to Cilium's IPsec feature feature/ipv6-only Relates to single-stack IPv6 support. needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Aug 1, 2025
@smagnani96
Copy link
Contributor Author

/test

@smagnani96 smagnani96 marked this pull request as ready for review August 5, 2025 08:48
@smagnani96 smagnani96 requested review from a team as code owners August 5, 2025 08:48
@smagnani96 smagnani96 requested a review from brlbil August 5, 2025 08:48
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 5, 2025
@joestringer joestringer added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit 2b35843 Aug 5, 2025
124 checks passed
@joestringer joestringer deleted the pr/smagnani96/fix-skip-upgrade-bpftrace branch August 5, 2025 16:27
@rastislavs rastislavs mentioned this pull request Aug 6, 2025
17 tasks
@rastislavs rastislavs added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Aug 6, 2025
@github-actions github-actions bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Aug 8, 2025
smagnani96 added a commit that referenced this pull request Aug 19, 2025
In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 19, 2025
In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
pippolo84 pushed a commit that referenced this pull request Aug 25, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84 pushed a commit that referenced this pull request Aug 25, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84 pushed a commit that referenced this pull request Aug 26, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84 pushed a commit that referenced this pull request Aug 26, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
pippolo84 pushed a commit that referenced this pull request Aug 26, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Sep 1, 2025
[ upstream commit 34242dd ]

In #40868, we observed no DNS traffic being recorded by the
check-encryption-leaks.bt script when skipping Cilium version downgrade
in IPv6-only cluster. This was true given the to-fqdn tests were run only
if IPv4 was enabled. However, the fix landed in #40881 is not enough:
the downgrade is skipped, but the chekc-encryption-leaks.bt script can
be still run in DNS-assertion mode for clusters with IPv4-enabled.
This would cause the script to throw an error if no DNS traffic is being
recorded. Given we skip the whole downgrade tests, there is no guarantee
that we see DNS traffic, given no CLI tests nor conn-disrupt tests run
at that moment in time.

There are two possible ways to fix that:

1. activate the DNS-assertion mode only when IPv4 is enabled (already doing this)
   AND when downgrade is not skipped.
2. skip the whole check-encryption-leaks.bt setup for the downgrade step
   when we're skipping downgrade (i.e., no tests would generate such traffic).

This commit opts for (2).

Fixes: #40868.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. feature/ipsec Relates to Cilium's IPsec feature feature/ipv6-only Relates to single-stack IPv6 support. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Cilium E2E Upgrade - ipsec-7 - assert that no unencrypted packets are leaked
4 participants