Skip to content

fix(controller): support simultaneous ALB and NGINX traffic routing validation#4460

Merged
zachaller merged 7 commits intoargoproj:masterfrom
puretension:fix/alb-nginx-simultaneous-routing-4452
Nov 4, 2025
Merged

fix(controller): support simultaneous ALB and NGINX traffic routing validation#4460
zachaller merged 7 commits intoargoproj:masterfrom
puretension:fix/alb-nginx-simultaneous-routing-4452

Conversation

@puretension
Copy link
Copy Markdown
Contributor

Summary

Fixes #4452 - This PR fixes a validation issue when both ALB and NGINX traffic routing are configured simultaneously in a Rollout. Previously, the validation logic incorrectly applied NGINX validation rules to ALB ingresses, causing validation errors.

Changes

  • Modified ValidateIngress function to check ingress ownership before applying validation rules
  • Added isNginxIngress() helper function to identify NGINX-managed ingresses
  • Added isAlbIngress() helper function to identify ALB-managed ingresses
  • Added comprehensive test cases for simultaneous ALB and NGINX usage scenarios

Testing

  • Added unit tests covering simultaneous ALB and NGINX configurations
  • Verified that ALB ingresses are not validated against NGINX rules
  • Verified that NGINX ingresses are not validated against ALB rules
  • All existing tests continue to pass

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

@puretension puretension changed the title fix(validation): support simultaneous ALB and NGINX traffic routing validation. Fixes #4452 fix(validation): support simultaneous ALB and NGINX traffic routing validation Sep 19, 2025
@puretension puretension force-pushed the fix/alb-nginx-simultaneous-routing-4452 branch 4 times, most recently from 987b331 to df186c8 Compare September 19, 2025 17:45
- Change jsonPath from {$.json} to {$} to get full response
- Update conditions to use result.json.status instead of result.status
- Fixes flaky TestCanaryInconclusiveBackgroundAnalysis e2e test

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
…alidation

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@puretension puretension force-pushed the fix/alb-nginx-simultaneous-routing-4452 branch from df186c8 to fb69697 Compare September 19, 2025 17:46
@puretension puretension changed the title fix(validation): support simultaneous ALB and NGINX traffic routing validation fix(controller): support simultaneous ALB and NGINX traffic routing validation Sep 19, 2025
Remove helper functions and use inline validation for cleaner code.
Reduces code complexity while maintaining the same functionality.

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
Reduce test code from 25 lines to 12 lines while maintaining coverage.

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 19, 2025

Published E2E Test Results

  4 files    4 suites   3h 50m 20s ⏱️
115 tests 103 ✅  7 💤  5 ❌
492 runs  433 ✅ 28 💤 31 ❌

For more details on these failures, see this check.

Results for commit 8e7c4f9.

♻️ This comment has been updated with latest results.

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 19, 2025

Published Unit Test Results

2 331 tests   2 331 ✅  3m 1s ⏱️
  129 suites      0 💤
    1 files        0 ❌

Results for commit 8e7c4f9.

♻️ This comment has been updated with latest results.

Use proper object creation before wrapping with NewLegacyIngress
instead of calling non-existent GetIngress() method.

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
- Fix line continuation formatting in validation logic
- Remove trailing whitespace in test comments

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

@zachaller zachaller merged commit 7965797 into argoproj:master Nov 4, 2025
24 checks passed
@fabioviana-hotmart
Copy link
Copy Markdown

Hi folks! When this improvement will be available on a GA version? Is there any release date planned?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

traffic routing by alb and nginx simultaneously is not working

3 participants