Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: secret reference update Ingress #1780

Merged
merged 9 commits into from
Apr 21, 2023

Conversation

lingsamuel
Copy link
Member

@lingsamuel lingsamuel commented Apr 11, 2023

Type of change:

  • Bugfix

Fixes #1777

What this PR does / why we need it:

Fix a bug that causes nil reference and secret change won't trigger Ingress resource sync.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@lingsamuel lingsamuel changed the title fix: secret reference update fix: secret reference update Ingress Apr 11, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2023

Codecov Report

Merging #1780 (b63667e) into master (3a8fdf6) will increase coverage by 0.11%.
The diff coverage is 0.00%.

❗ Current head b63667e differs from pull request most recent head 0335372. Consider uploading reports for the commit 0335372 to get more accurate results

@@            Coverage Diff             @@
##           master    #1780      +/-   ##
==========================================
+ Coverage   39.30%   39.42%   +0.11%     
==========================================
  Files          91       91              
  Lines        8073     8049      -24     
==========================================
  Hits         3173     3173              
+ Misses       4494     4469      -25     
- Partials      406      407       +1     
Impacted Files Coverage Δ
pkg/apisix/cluster.go 28.14% <ø> (ø)
...kg/providers/apisix/translation/apisix_consumer.go 65.21% <0.00%> (-1.95%) ⬇️
pkg/providers/ingress/ingress.go 4.98% <0.00%> (+0.44%) ⬆️
pkg/providers/ingress/translation/translator.go 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
pkg/providers/ingress/ingress.go Outdated Show resolved Hide resolved
pkg/providers/ingress/ingress.go Outdated Show resolved Hide resolved
@AlinsRan
Copy link
Contributor

When deleting a secret, does the SSL object of APISIX exist?
Missing e2e case for delete-secret?

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@tao12345666333 tao12345666333 merged commit a414df7 into apache:master Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

bug: Deleting secret when using ingress tls will cause controller to crash
4 participants