secret-sync: extract secret-sync logic from gateway api controller & introduce hive cell #29100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
9b1d854 to
d850dce
Compare
|
/test |
|
/test |
bimmlerd
reviewed
Nov 13, 2023
d850dce to
9b7c82a
Compare
Thanks a lot for your valuable feedback @bimmlerd ! I tried to address and answer your concerns / questions! |
bimmlerd
approved these changes
Nov 13, 2023
|
/test |
In preparation for reusing the secretsyncer for Ingress too, this commit removes the Gateway API related field `controllerName` from the struct `secretSyncer`. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
As a preparation for extracting the secretSyncer into its own package, this commit adds the logger as field to the secretSyncer. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
In preparation to extract the secret syncer into its own package, the Gateway API related GatewayAPI watch defined in the method `enqueueTLSSecrets` has been converted into a function. This way it can be passed into the secretSyncer when used by the package gatewayapi. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit changes the secret-sync label prefix for owning namespace and name from `io.cilium.gateway` to `secretsync.cilium.io`. This way the same label can be re-used for Cilium Ingress secret sync. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit removes the unused field scheme from the secretSyncer. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
In preparation to re-use the secret-syncer from Gateway API for Cilium Ingress, the mainObject (e.g. Gateway) and mainObjectEnqueueFunc (how to map changed Gateways to referenced Secrets) are represented as field in the `secretSyncer` and passed on creation. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit extracts the function mainObjectReferencedFunc as field of the `secretSyncer` to pass this function into the secret syncer. This enables to re-use the secretsyncer for Cilium Ingress. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit moves the reconcile helper functions (fail & success) into the controller-runtime package. This way they can be used by multiple reconcilers over different packages. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit extracts the secret-sync related Gateway API functions into a dedicated file. This way the secret-sync can be moved into its own package. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit extracts the secretSyncer into its own package. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit introduces a dedicated cell for the TLS Secret Sync. Cells that are interested in having TLS Secrets synced (e.g. Gateway API) register themself as SecretSyncRegistration by providing information about which Secrets are required to sync. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit moves the unit-test of the secret-sync reconciliation back into the secretsync package. To be able to use the Gateway without introducing a cyclic dependency, a dedicated test package `secretsync_test` is used. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
With this commit, the usage of the global logger in `EnqueueTLSSecrets` gets replaced with a logger passed as function argument. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
9b7c82a to
383fae6
Compare
|
rebased to |
|
/test |
mhofstetter
added a commit
to mhofstetter/cilium
that referenced
this pull request
Nov 28, 2023
With cilium#28982 a check got introduced that disables the Gateway API functionality if the required Gateway API CRDs aren't installed in the cluster. But this check is not executed when the secret sync registrations is performed. This results in failures in the secret-sync: ``` level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime ``` Therefore, this commit adds the check to the secret sync registration too. Fixes: cilium#29100 Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 30, 2023
With #28982 a check got introduced that disables the Gateway API functionality if the required Gateway API CRDs aren't installed in the cluster. But this check is not executed when the secret sync registrations is performed. This results in failures in the secret-sync: ``` level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime ``` Therefore, this commit adds the check to the secret sync registration too. Fixes: #29100 Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
pjablonski123
pushed a commit
to pjablonski123/cilium
that referenced
this pull request
Dec 15, 2023
With cilium#28982 a check got introduced that disables the Gateway API functionality if the required Gateway API CRDs aren't installed in the cluster. But this check is not executed when the secret sync registrations is performed. This results in failures in the secret-sync: ``` level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime ``` Therefore, this commit adds the check to the secret sync registration too. Fixes: cilium#29100 Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up of #29017
In preparation for migrating the Cilium Ingress controller to use the controller-runtime library (#28911), the K8s TLS Secret synchronization logic from the Gateway API controller gets untangled from the rest of Gateway API logic and extracted into its own Hive Cell. Dependent cells can register their need for having Secrets synced.
This way, the Cilium Ingress controller can re-use the existing logic that already uses the controller-runtime library. (will be done in an upcoming PR).
For more information, please refer to the individual commits.