-
Notifications
You must be signed in to change notification settings - Fork 350
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: ssl need to be update when secret has been changed #337
Conversation
@gxthrj Now we only verify that data are passed to the DP but the verifications of the use of these certificates are missing. |
Yes, you are right, I will add it. |
Codecov Report
@@ Coverage Diff @@
## master #337 +/- ##
==========================================
- Coverage 43.98% 43.47% -0.51%
==========================================
Files 40 39 -1
Lines 3456 3457 +1
==========================================
- Hits 1520 1503 -17
- Misses 1767 1783 +16
- Partials 169 171 +2
Continue to review full report at Codecov.
|
test/e2e/scaffold/scaffold.go
Outdated
@@ -175,6 +179,17 @@ func (s *Scaffold) NewAPISIXHttpsClient() *httpexpect.Expect { | |||
// accept any certificate; for testing only! | |||
InsecureSkipVerify: true, | |||
}, | |||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not set the ServerName
in tls.Config
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, use ServerName
instead.
test/e2e/ingress/secret.go
Outdated
DualStack: true, | ||
} | ||
|
||
http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think DialContext
can also be removed if we already have the correct ServerName
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, dirty codes.
Please answer these questions before submitting a pull request
Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
bug: secret controller doesn't update ApisixTls but just re-push the old one #324