Fix 154: adding a finalizer to the tunnel's associated secret#158
Open
cyclingwithelephants wants to merge 11 commits intoadyanth:mainfrom
Open
Fix 154: adding a finalizer to the tunnel's associated secret#158cyclingwithelephants wants to merge 11 commits intoadyanth:mainfrom
cyclingwithelephants wants to merge 11 commits intoadyanth:mainfrom
Conversation
cyclingwithelephants
force-pushed
the
154-secret-finaliser
branch
from
5a59e2d to
ab0cbbc
Compare
cyclingwithelephants
force-pushed
the
154-secret-finaliser
branch
from
37ae1d7 to
5e48ba2
Compare
Contributor
Author
|
Looks like coverage reports are not available from forks |
Owner
|
To answer your question, yes the namespace for the cluster tunnel is the operator namespace. |
cyclingwithelephants
force-pushed
the
154-secret-finaliser
branch
from
c6bcca2 to
de18169
Compare
Contributor
Author
|
Haven't tested yet, but please let me know if you're otherwise happy with the changes |
cyclingwithelephants
force-pushed
the
154-secret-finaliser
branch
from
de18169 to
0a478e6
Compare
adyanth
reviewed
May 8, 2025
Owner
adyanth
left a comment
adyanth
left a comment
There was a problem hiding this comment.
Couple of suggestions, looks good otherwise!
Contributor
Author
|
this is functionally working, but I've noticed that the controller is producing errors on deletion. This wasn't happening when using the concrete implementation for the secret. What's strange is that this |
adyanth
reviewed
May 9, 2025
Comment on lines
148
to
153
| if apierrors.IsNotFound(err) { | ||
| // Tunnel object not found, could have been deleted after reconcile request. | ||
| // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. | ||
| // Return and don't requeue | ||
| r.log.Info("Tunnel deleted, nothing to do") | ||
| // Owned objects are automatically garbage collected. | ||
| objectClient, err := k8s.NewObjectClient(r.Client, &r.log) | ||
| if err != nil { | ||
| return ctrl.Result{}, err |
Owner
There was a problem hiding this comment.
This logic is in the wrong place (ref: the error you saw). When this condition is hit, the tunnel is already removed from etcd.
What you need is, instead of the logic in both here and tunnel_controller.go, move this to
cloudflare-operator/internal/controller/generic_tunnel_reconciler.go
Lines 157 to 162 in d1570e5
Contributor
Author
There was a problem hiding this comment.
Oh, derp. Thanks will fix this
Owner
There was a problem hiding this comment.
Did you get around to this? Would love to merge this in
Contributor
Author
There was a problem hiding this comment.
I haven't sorry, life has been a lot this past month! Hoping to get to this next weekend
Contributor
Author
There was a problem hiding this comment.
Just wanted to give you another heads up, might be another week
StringKe
referenced
this pull request
in StringKe/cloudflare-operator
Jan 7, 2026
Implement comprehensive Cloudflare Zero Trust Kubernetes operator with 18 CRDs: **New CRDs:** - AccessApplication: Zero Trust application definitions - AccessGroup: Reusable access policy groups - AccessIdentityProvider: IdP configurations (OIDC, SAML, GitHub, Azure AD) - AccessServiceToken: Machine-to-machine authentication tokens - VirtualNetwork: Cloudflare virtual networks for traffic isolation - NetworkRoute: IP routes through tunnels to private networks - PrivateService: Expose K8s Services via WARP private IPs - GatewayRule: Gateway DNS/HTTP/network policies - GatewayList: Lists for gateway policy rules - GatewayConfiguration: Global gateway settings - DeviceSettingsPolicy: WARP client settings and split tunnels - DevicePostureRule: Device posture checks for Zero Trust - DNSRecord: DNS record management - WARPConnector: WARP connector deployments **Enhancements:** - Add EnableWarpRouting to Tunnel/ClusterTunnel for private network access - Add cluster-resource-namespace flag with Downward API injection (PR #178) - Store previous-hostnames in TunnelBinding annotation (PR #166) - Fix Secret finalizer order in cleanupTunnel (PR #158) **API Clients:** - Access API: Applications, Groups, Identity Providers, Service Tokens - Gateway API: Rules, Lists, Configurations - Device API: Split Tunnel, Fallback Domains, Posture Rules - Network API: Virtual Networks, Routes - DNS API: Record management 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
StringKe
referenced
this pull request
in StringKe/cloudflare-operator
Jan 7, 2026
Release v0.14.0 with Zero Trust CRDs: - 14 new CRDs for Access, Gateway, Device, and Network management - WARP routing support for private network access - Upstream PR fixes (#178, #166, #158) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
fixes #154
I just wanted to confirm that
r.GetTunnel().GetNamespace()for the ClusterTunnel resource is expected to return the cloudflare-operator's namespace? I've tested this working, just wanted to make sure this was built on expected behaviour and wasn't a fluke