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

ingress: Delay secret sync if not available #26988

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

sayboras
Copy link
Member

Currently, if the TLS secret is not available by the time the Ingress object is created, the creation event will be filtered out based on in-memory list of watched secrets. This commit is to make sure that we still add secret name to watch list, so that creation event will trigger the sync accordingly.

Testing is done as per below:

$ curl -k -v https://bookinfo.cilium.rocks/details/1
*   Trying 10.104.207.106:443...
* Connected to bookinfo.cilium.rocks (10.104.207.106) port 443 (#0)
* ALPN: offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* Recv failure: Connection reset by peer
* OpenSSL SSL_connect: Connection reset by peer in connection to bookinfo.cilium.rocks:443
* Closing connection 0
curl: (35) Recv failure: Connection reset by peer
$ kubectl create secret tls demo-cert --key=_.cilium.rocks/key.pem --cert=_.cilium.rocks/cert.pem
secret/demo-cert created
$ curl -k https://bookinfo.cilium.rocks/details/1
{"id":1,"author":"William Shakespeare","year":1595,"type":"paperback","pages":200,"publisher":"PublisherA","language":"English","ISBN-10":"1234567890","ISBN-13":"123-1234567890"}

Currently, if the TLS secret is not available by the time the Ingress
object is created, the creation event will be filtered out based on
in-memory list of watched secrets. This commit is to make sure that
we still add secret name to watch list, so that creation event will
trigger the sync accordingly.

Testing is done as per below:

```
$ curl -k -v https://bookinfo.cilium.rocks/details/1
*   Trying 10.104.207.106:443...
* Connected to bookinfo.cilium.rocks (10.104.207.106) port 443 (#0)
* ALPN: offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* Recv failure: Connection reset by peer
* OpenSSL SSL_connect: Connection reset by peer in connection to bookinfo.cilium.rocks:443
* Closing connection 0
curl: (35) Recv failure: Connection reset by peer
$ kubectl create secret tls demo-cert --key=_.cilium.rocks/key.pem --cert=_.cilium.rocks/cert.pem
secret/demo-cert created
$ curl -k https://bookinfo.cilium.rocks/details/1
{"id":1,"author":"William Shakespeare","year":1595,"type":"paperback","pages":200,"publisher":"PublisherA","language":"English","ISBN-10":"1234567890","ISBN-13":"123-1234567890"}
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 21, 2023
@sayboras sayboras added release-note/bug This PR fixes an issue in a previous release of Cilium. feature/k8s-ingress labels Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 21, 2023
@sayboras sayboras added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. needs-backport/1.12 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.0 Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.5 Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.12 Jul 21, 2023
@sayboras sayboras marked this pull request as ready for review July 21, 2023 13:44
@sayboras sayboras requested a review from a team as a code owner July 21, 2023 13:44
@sayboras sayboras requested a review from meyskens July 21, 2023 13:44
@sayboras
Copy link
Member Author

/test

Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@sayboras
Copy link
Member Author

Review is done by the required team, and CI is all good, merging this one in.

@sayboras sayboras merged commit 301844c into cilium:main Jul 21, 2023
66 checks passed
@sayboras sayboras deleted the tam/secret-after-ingress branch July 21, 2023 14:59
@sayboras sayboras mentioned this pull request Jul 21, 2023
1 task
@sayboras sayboras added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.5 Jul 21, 2023
@sayboras sayboras mentioned this pull request Jul 21, 2023
1 task
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.12 in 1.12.12 Jul 21, 2023
@sayboras sayboras mentioned this pull request Jul 21, 2023
1 task
@sayboras sayboras added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 21, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.0 Jul 21, 2023
@sayboras sayboras added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.12 backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.0 Jul 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.5 Jul 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.5 Jul 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.12 Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/k8s-ingress release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.12.12
Backport done to v1.12
1.13.5
Backport done to v1.13
1.14.0
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

2 participants