Skip to content

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 requested a review from sayboras November 7, 2023 10:19
@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
Contributor

@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

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
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.

4 participants