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

gateway-api: improve secret sync resiliency #29017

Merged

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Nov 7, 2023

This PR improves the resiliency of the Gateway API secret sync.

main changes:

  • delete synced secret if source secret gets deleted
  • delete synced secret if source secret is no longer referenced by a Cilium Gateway
  • trigger reconcile when the synced secret gets deleted or updated

Please review the individual commits.

This is a preparation to re-use the secret synchronization for Cilium Ingress (#28911)

@mhofstetter mhofstetter added kind/enhancement This would improve or streamline existing functionality. release-note/misc This PR makes changes that have no direct user impact. area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api labels Nov 7, 2023
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/gateway-api-secret-sync branch from e71ac5f to 6ed8e0a Compare November 7, 2023 10:00
@mhofstetter
Copy link
Member Author

/test

1 similar comment
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter marked this pull request as ready for review November 7, 2023 10:19
@mhofstetter mhofstetter requested a review from a team as a code owner November 7, 2023 10:19
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/gateway-api-secret-sync branch from 6ed8e0a to f066116 Compare November 9, 2023 14:53
This commit removes the predicate option when watching Secrets in the
secret-syncer.

Without this change we miss to reconcile when:
* the synced secret (in NS cilium-secrets) gets deleted or updated
* the source secret gets deleted and might no longer be referenced
  by a Gateway resource

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit restricts the Gateway API's secret sync `For` watch
to the source secrets that are outside of the target secrets namespace.

This way, already synced secrets aren't re-synced (endless-reconciliation).

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
Copy link
Member Author

updated the test-fixtures

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/gateway-api-secret-sync branch from f066116 to 6b9a46b Compare November 9, 2023 16:34
@mhofstetter
Copy link
Member Author

mhofstetter commented Nov 9, 2023

/test

after rebasing to main

Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

LGTM just one small thing for readability but giving you a green tick anyway :D

operator/pkg/gateway-api/secret.go Outdated Show resolved Hide resolved
Currently, Gateway API secret sync reconciliation isn't triggered
when a synced k8s Secret in the secrets namespace (defaults to
`cilium-secrets`) gets deleted or updated.

Therefore, this commit introduces an additional watcher that
watches for updated & deleted secrets and triggers a reconciliation
of the owning Secret by using the labels
`io.cilium.gateway/owning-secret-namespace` &
`io.cilium.gateway/owning-secret-name` on the synced Secret.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit moves the controller-check (whether a Gateway is managed
by Cilium Operator) from the Predicate into the EventHandler.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit unexports the fields `client`, `scheme` & `secretsNamespace`
from the type `secretSyncer`.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit refactors the Gateway API Secret Sync to handle
deleted source secrets.

Whenever a Secret gets deleted, the corresponding synced secret
gets deleted too.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit refactors the Gateway API Secret Sync reconciler
to take the referencing Gateway TLS secret references into account.

Meaning that a Secret only gets synced once its referenced by a Gateway
resource that is managed by Cilium.

In addition, the synced secrets gets deleted once the last reference is
deleted.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit introduces a unit test for the secret-sync reconcilation.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/gateway-api-secret-sync branch from 6b9a46b to b90c021 Compare November 10, 2023 09:39
@mhofstetter
Copy link
Member Author

Addressed @meyskens input - Thanks for the review!

@mhofstetter
Copy link
Member Author

/test

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Thanks ✔️

@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 Nov 10, 2023
@squeed squeed merged commit fd20d3f into cilium:main Nov 10, 2023
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants