diff --git a/README.md b/README.md index cf9893bb..923cd4df 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,8 @@ cloudflare-operator helps to: - Keep Cloudflare DNS records up to date - Update your external IP address on Cloudflare DNS records +> **Heads-up:** every `Account` custom resource must list the zones it is allowed to manage in `spec.managedZones`. Zones and DNS records are matched to accounts automatically using this list. + ## What can I do with cloudflare-operator? cloudflare-operator is based on a set of Kubernetes API extensions ("custom resources"), which control Cloudflare DNS records. diff --git a/api/v1/account_types.go b/api/v1/account_types.go index 54815947..7bb7d859 100644 --- a/api/v1/account_types.go +++ b/api/v1/account_types.go @@ -35,9 +35,8 @@ type AccountSpec struct { // +optional Interval metav1.Duration `json:"interval,omitempty"` // List of zone names that should be managed by cloudflare-operator - // Deprecated and will be removed in a future release + // Used to automatically match zones with the correct account // +optional - // +deprecated ManagedZones []string `json:"managedZones,omitempty"` } diff --git a/cmd/main.go b/cmd/main.go index 7b14dab4..0f80fc1a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -35,7 +35,6 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" - "github.com/cloudflare/cloudflare-go" cloudflareoperatoriov1 "github.com/containeroo/cloudflare-operator/api/v1" "github.com/containeroo/cloudflare-operator/internal/controller" // +kubebuilder:scaffold:imports @@ -65,7 +64,6 @@ func main() { retryInterval time.Duration ipReconcilerHTTPClientTimeout time.Duration defaultReconcileInterval time.Duration - cloudflareAPI cloudflare.API ctx = ctrl.SetupSignalHandler() ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") @@ -128,20 +126,22 @@ func main() { os.Exit(1) } + accountManager := controller.NewAccountManager() + if err = (&controller.AccountReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - CloudflareAPI: &cloudflareAPI, - RetryInterval: retryInterval, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + AccountManager: accountManager, + RetryInterval: retryInterval, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Account") os.Exit(1) } if err = (&controller.ZoneReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - CloudflareAPI: &cloudflareAPI, - RetryInterval: retryInterval, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + AccountManager: accountManager, + RetryInterval: retryInterval, }).SetupWithManager(ctx, mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Zone") os.Exit(1) @@ -166,10 +166,10 @@ func main() { os.Exit(1) } if err = (&controller.DNSRecordReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - CloudflareAPI: &cloudflareAPI, - RetryInterval: retryInterval, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + AccountManager: accountManager, + RetryInterval: retryInterval, }).SetupWithManager(ctx, mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DNSRecord") os.Exit(1) diff --git a/config/crd/bases/cloudflare-operator.io_accounts.yaml b/config/crd/bases/cloudflare-operator.io_accounts.yaml index a2c6f9a7..3fac5c96 100644 --- a/config/crd/bases/cloudflare-operator.io_accounts.yaml +++ b/config/crd/bases/cloudflare-operator.io_accounts.yaml @@ -70,7 +70,7 @@ spec: managedZones: description: |- List of zone names that should be managed by cloudflare-operator - Deprecated and will be removed in a future release + Used to automatically match zones with the correct account items: type: string type: array diff --git a/config/samples/cloudflareoperatorio_v1_account.yaml b/config/samples/cloudflareoperatorio_v1_account.yaml index c0c570ec..be7ff194 100644 --- a/config/samples/cloudflareoperatorio_v1_account.yaml +++ b/config/samples/cloudflareoperatorio_v1_account.yaml @@ -17,3 +17,5 @@ spec: secretRef: name: api-token-sample namespace: cloudflare-operator-system + managedZones: + - containeroo-test.org diff --git a/internal/controller/account_controller.go b/internal/controller/account_controller.go index dc73f9f1..56e3c26a 100644 --- a/internal/controller/account_controller.go +++ b/internal/controller/account_controller.go @@ -48,7 +48,7 @@ type AccountReconciler struct { RetryInterval time.Duration - CloudflareAPI *cloudflare.API + AccountManager *AccountManager } var errWaitForAccount = errors.New("must wait for account") @@ -123,16 +123,14 @@ func (r *AccountReconciler) reconcileAccount(ctx context.Context, account *cloud return ctrl.Result{RequeueAfter: r.RetryInterval}, nil } - if r.CloudflareAPI.APIToken != cloudflareAPIToken { - cloudflareAPI, err := cloudflare.NewWithAPIToken(cloudflareAPIToken) - if err != nil { - intconditions.MarkFalse(account, err) - return ctrl.Result{}, err - } - - *r.CloudflareAPI = *cloudflareAPI + cloudflareAPI, err := cloudflare.NewWithAPIToken(cloudflareAPIToken) + if err != nil { + intconditions.MarkFalse(account, err) + return ctrl.Result{}, err } + r.AccountManager.UpsertAccount(account.Name, cloudflareAPI, cloudflareAPIToken, account.Spec.ManagedZones) + intconditions.MarkTrue(account, "Account is ready") return ctrl.Result{RequeueAfter: account.Spec.Interval.Duration}, nil @@ -142,4 +140,5 @@ func (r *AccountReconciler) reconcileAccount(ctx context.Context, account *cloud func (r *AccountReconciler) reconcileDelete(account *cloudflareoperatoriov1.Account) { metrics.AccountFailureCounter.DeleteLabelValues(account.Name) controllerutil.RemoveFinalizer(account, cloudflareoperatoriov1.CloudflareOperatorFinalizer) + r.AccountManager.RemoveAccount(account.Name) } diff --git a/internal/controller/account_controller_test.go b/internal/controller/account_controller_test.go index f59324be..039f771a 100644 --- a/internal/controller/account_controller_test.go +++ b/internal/controller/account_controller_test.go @@ -31,7 +31,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/cloudflare/cloudflare-go" cloudflareoperatoriov1 "github.com/containeroo/cloudflare-operator/api/v1" networkingv1 "k8s.io/api/networking/v1" ) @@ -44,8 +43,6 @@ func NewTestScheme() *runtime.Scheme { return s } -var cloudflareAPI cloudflare.API - func TestAccountReconciler_reconcileAccount(t *testing.T) { t.Run("reconcile account", func(t *testing.T) { g := NewWithT(t) @@ -74,12 +71,14 @@ func TestAccountReconciler_reconcileAccount(t *testing.T) { }, } + accountManager := NewAccountManager() + r := &AccountReconciler{ Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithObjects(secret, account). Build(), - CloudflareAPI: &cloudflareAPI, + AccountManager: accountManager, } _, err := r.reconcileAccount(context.TODO(), account) @@ -89,7 +88,10 @@ func TestAccountReconciler_reconcileAccount(t *testing.T) { *conditions.TrueCondition(cloudflareoperatoriov1.ConditionTypeReady, cloudflareoperatoriov1.ConditionReasonReady, "Account is ready"), })) - g.Expect(cloudflareAPI.APIToken).To(Equal(string(secret.Data["apiToken"]))) + api, token, ok := accountManager.GetAccount(account.Name) + g.Expect(ok).To(BeTrue()) + g.Expect(token).To(Equal(string(secret.Data["apiToken"]))) + g.Expect(api).ToNot(BeNil()) }) t.Run("econcile account error secret not found", func(t *testing.T) { @@ -109,12 +111,14 @@ func TestAccountReconciler_reconcileAccount(t *testing.T) { }, } + accountManager := NewAccountManager() + r := &AccountReconciler{ Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithObjects(account). Build(), - CloudflareAPI: &cloudflareAPI, + AccountManager: accountManager, } _, err := r.reconcileAccount(context.TODO(), account) @@ -152,12 +156,14 @@ func TestAccountReconciler_reconcileAccount(t *testing.T) { }, } + accountManager := NewAccountManager() + r := &AccountReconciler{ Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithObjects(secret, account). Build(), - CloudflareAPI: &cloudflareAPI, + AccountManager: accountManager, } _, err := r.reconcileAccount(context.TODO(), account) diff --git a/internal/controller/account_manager.go b/internal/controller/account_manager.go new file mode 100644 index 00000000..621de69a --- /dev/null +++ b/internal/controller/account_manager.go @@ -0,0 +1,173 @@ +package controller + +import ( + "errors" + "sort" + "strings" + "sync" + + "github.com/cloudflare/cloudflare-go" +) + +var ( + errNoAccountForZone = errors.New("no account manages the requested zone") + errMultipleAccounts = errors.New("multiple accounts manage the requested zone") +) + +// accountInfo holds the data we need to interact with Cloudflare for a single account. +type accountInfo struct { + api *cloudflare.API + managedZones map[string]struct{} + token string +} + +// AccountManager keeps track of available Cloudflare accounts and the zones they manage. +type AccountManager struct { + mu sync.RWMutex + accounts map[string]*accountInfo + zoneToAccounts map[string]map[string]struct{} +} + +// NewAccountManager returns an initialized AccountManager instance. +func NewAccountManager() *AccountManager { + return &AccountManager{ + accounts: make(map[string]*accountInfo), + zoneToAccounts: make(map[string]map[string]struct{}), + } +} + +// UpsertAccount registers or updates an account and its managed zones. +func (m *AccountManager) UpsertAccount(name string, api *cloudflare.API, token string, managedZones []string) { + canonicalZones := normalizeZones(managedZones) + + m.mu.Lock() + defer m.mu.Unlock() + + // remove old zone membership for this account + if existing, ok := m.accounts[name]; ok { + for zone := range existing.managedZones { + m.removeZoneMembership(zone, name) + } + } + + zoneSet := make(map[string]struct{}, len(canonicalZones)) + for _, zone := range canonicalZones { + zoneSet[zone] = struct{}{} + if _, ok := m.zoneToAccounts[zone]; !ok { + m.zoneToAccounts[zone] = make(map[string]struct{}) + } + m.zoneToAccounts[zone][name] = struct{}{} + } + + m.accounts[name] = &accountInfo{ + api: api, + managedZones: zoneSet, + token: token, + } +} + +// RemoveAccount unregisters an account and clears any zone mappings. +func (m *AccountManager) RemoveAccount(name string) { + m.mu.Lock() + defer m.mu.Unlock() + + entry, ok := m.accounts[name] + if !ok { + return + } + + for zone := range entry.managedZones { + m.removeZoneMembership(zone, name) + } + + delete(m.accounts, name) +} + +// GetAccount returns the stored account info by name. +func (m *AccountManager) GetAccount(name string) (*cloudflare.API, string, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + + entry, ok := m.accounts[name] + if !ok { + return nil, "", false + } + + return entry.api, entry.token, true +} + +// AccountForZone returns the Cloudflare client for the account managing the provided zone. +// If no account manages the zone, errNoAccountForZone is returned. +// If multiple accounts manage the same zone, errMultipleAccounts is returned along with the list of candidates. +func (m *AccountManager) AccountForZone(zone string) (*cloudflare.API, string, error) { + canonical := canonicalZone(zone) + + m.mu.RLock() + defer m.mu.RUnlock() + + accounts, ok := m.zoneToAccounts[canonical] + if !ok || len(accounts) == 0 { + return nil, "", errNoAccountForZone + } + + if len(accounts) > 1 { + names := make([]string, 0, len(accounts)) + for name := range accounts { + names = append(names, name) + } + sort.Strings(names) + return nil, strings.Join(names, ", "), errMultipleAccounts + } + + var accountName string + for name := range accounts { + accountName = name + } + + entry, ok := m.accounts[accountName] + if !ok { + return nil, accountName, errNoAccountForZone + } + + return entry.api, accountName, nil +} + +func (m *AccountManager) removeZoneMembership(zone, accountName string) { + canonical := canonicalZone(zone) + members, ok := m.zoneToAccounts[canonical] + if !ok { + return + } + + delete(members, accountName) + if len(members) == 0 { + delete(m.zoneToAccounts, canonical) + } +} + +func normalizeZones(zones []string) []string { + result := make([]string, 0, len(zones)) + seen := make(map[string]struct{}, len(zones)) + for _, zone := range zones { + canonical := canonicalZone(zone) + if canonical == "" { + continue + } + if _, ok := seen[canonical]; ok { + continue + } + seen[canonical] = struct{}{} + result = append(result, canonical) + } + + sort.Strings(result) + return result +} + +func canonicalZone(zone string) string { + zone = strings.TrimSpace(zone) + if zone == "" { + return "" + } + return strings.ToLower(zone) +} diff --git a/internal/controller/dnsrecord_controller.go b/internal/controller/dnsrecord_controller.go index 7bcdf258..1dc7eb0b 100644 --- a/internal/controller/dnsrecord_controller.go +++ b/internal/controller/dnsrecord_controller.go @@ -54,7 +54,7 @@ type DNSRecordReconciler struct { RetryInterval time.Duration - CloudflareAPI *cloudflare.API + AccountManager *AccountManager } // SetupWithManager sets up the controller with the Manager. @@ -143,8 +143,22 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( zone := &zones.Items[0] + cloudflareAPI, accountName, err := r.AccountManager.AccountForZone(zone.Spec.Name) + if errors.Is(err, errNoAccountForZone) { + intconditions.MarkFalse(dnsrecord, fmt.Errorf("no Cloudflare account manages zone %q", zone.Spec.Name)) + return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount + } + if errors.Is(err, errMultipleAccounts) { + intconditions.MarkFalse(dnsrecord, fmt.Errorf("multiple Cloudflare accounts manage zone %q: %s", zone.Spec.Name, accountName)) + return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount + } + if err != nil { + intconditions.MarkFalse(dnsrecord, err) + return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount + } + if !dnsrecord.DeletionTimestamp.IsZero() { - if err := r.reconcileDelete(ctx, zone.Status.ID, dnsrecord); err != nil { + if err := r.reconcileDelete(ctx, cloudflareAPI, zone.Status.ID, dnsrecord); err != nil { log.Error(err, "Failed to delete DNS record in Cloudflare, record may still exist in Cloudflare") return ctrl.Result{}, err } @@ -156,16 +170,11 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{Requeue: true}, nil } - return r.reconcileDNSRecord(ctx, dnsrecord, zone) + return r.reconcileDNSRecord(ctx, dnsrecord, zone, cloudflareAPI) } // reconcileDNSRecord reconciles the dnsrecord -func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, zone *cloudflareoperatoriov1.Zone) (ctrl.Result, error) { - if r.CloudflareAPI.APIToken == "" { - intconditions.MarkUnknown(dnsrecord, "Cloudflare account is not ready") - return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount - } - +func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord *cloudflareoperatoriov1.DNSRecord, zone *cloudflareoperatoriov1.Zone, cloudflareAPI *cloudflare.API) (ctrl.Result, error) { if !conditions.IsTrue(zone, cloudflareoperatoriov1.ConditionTypeReady) { intconditions.MarkUnknown(dnsrecord, "Zone is not ready") return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForZone @@ -174,13 +183,13 @@ func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord var existingRecord cloudflare.DNSRecord if dnsrecord.Status.RecordID != "" { var err error - existingRecord, err = r.CloudflareAPI.GetDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), dnsrecord.Status.RecordID) + existingRecord, err = cloudflareAPI.GetDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), dnsrecord.Status.RecordID) if err != nil { intconditions.MarkFalse(dnsrecord, err) return ctrl.Result{RequeueAfter: r.RetryInterval}, nil } } else { - cloudflareExistingRecord, _, err := r.CloudflareAPI.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{ + cloudflareExistingRecord, _, err := cloudflareAPI.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{ Type: dnsrecord.Spec.Type, Name: dnsrecord.Spec.Name, Content: dnsrecord.Spec.Content, @@ -210,7 +219,7 @@ func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord } if existingRecord.ID == "" { - newDNSRecord, err := r.CloudflareAPI.CreateDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.CreateDNSRecordParams{ + newDNSRecord, err := cloudflareAPI.CreateDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.CreateDNSRecordParams{ Name: dnsrecord.Spec.Name, Type: dnsrecord.Spec.Type, Content: dnsrecord.Spec.Content, @@ -226,7 +235,7 @@ func (r *DNSRecordReconciler) reconcileDNSRecord(ctx context.Context, dnsrecord } dnsrecord.Status.RecordID = newDNSRecord.ID } else if !r.compareDNSRecord(dnsrecord.Spec, existingRecord) { - if _, err := r.CloudflareAPI.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.UpdateDNSRecordParams{ + if _, err := cloudflareAPI.UpdateDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.UpdateDNSRecordParams{ ID: dnsrecord.Status.RecordID, Name: dnsrecord.Spec.Name, Type: dnsrecord.Spec.Type, @@ -332,8 +341,8 @@ func (r *DNSRecordReconciler) requestsForIPChange(ctx context.Context, o client. } // reconcileDelete reconciles the deletion of the dnsrecord -func (r *DNSRecordReconciler) reconcileDelete(ctx context.Context, zoneID string, dnsrecord *cloudflareoperatoriov1.DNSRecord) error { - if err := r.CloudflareAPI.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zoneID), dnsrecord.Status.RecordID); err != nil && err.Error() != "Record does not exist. (81044)" && dnsrecord.Status.RecordID != "" { +func (r *DNSRecordReconciler) reconcileDelete(ctx context.Context, cloudflareAPI *cloudflare.API, zoneID string, dnsrecord *cloudflareoperatoriov1.DNSRecord) error { + if err := cloudflareAPI.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zoneID), dnsrecord.Status.RecordID); err != nil && err.Error() != "Record does not exist. (81044)" && dnsrecord.Status.RecordID != "" { return err } metrics.DnsRecordFailureCounter.DeleteLabelValues(dnsrecord.Namespace, dnsrecord.Name, dnsrecord.Spec.Name) diff --git a/internal/controller/dnsrecord_controller_test.go b/internal/controller/dnsrecord_controller_test.go index 2b1fcbf0..56c9519a 100644 --- a/internal/controller/dnsrecord_controller_test.go +++ b/internal/controller/dnsrecord_controller_test.go @@ -34,6 +34,14 @@ import ( ) func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { + g := NewWithT(t) + + apiToken := os.Getenv("CF_API_TOKEN") + g.Expect(apiToken).ToNot(BeEmpty(), "CF_API_TOKEN must be set for this test") + + cloudflareAPI, err := cloudflare.NewWithAPIToken(apiToken) + g.Expect(err).ToNot(HaveOccurred()) + zone := &cloudflareoperatoriov1.Zone{ ObjectMeta: metav1.ObjectMeta{ Name: "zone", @@ -51,6 +59,10 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { }}, }, } + g.Expect(zone.Status.ID).ToNot(BeEmpty(), "CF_ZONE_ID must be set for this test") + + accountManager := NewAccountManager() + accountManager.UpsertAccount("account", cloudflareAPI, apiToken, []string{zone.Spec.Name}) dnsRecord := &cloudflareoperatoriov1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ @@ -73,7 +85,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { WithScheme(NewTestScheme()). WithObjects(dnsRecord, ip). Build(), - CloudflareAPI: &cloudflareAPI, + AccountManager: accountManager, } t.Run("reconcile dnsrecord", func(t *testing.T) { @@ -85,7 +97,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { Proxied: new(bool), } - _, err := r.reconcileDNSRecord(context.TODO(), dnsRecord, zone) + _, err := r.reconcileDNSRecord(context.TODO(), dnsRecord, zone, cloudflareAPI) g.Expect(err).ToNot(HaveOccurred()) g.Expect(dnsRecord.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -97,7 +109,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { g.Expect(dnsRecord.Status.RecordID).To(Equal(cloudflareDNSRecord.ID)) - _ = r.reconcileDelete(context.TODO(), zone.Status.ID, dnsRecord) + _ = r.reconcileDelete(context.TODO(), cloudflareAPI, zone.Status.ID, dnsRecord) _, err = cloudflareAPI.GetDNSRecord(context.TODO(), cloudflare.ZoneIdentifier(zone.Status.ID), dnsRecord.Status.RecordID) g.Expect(err.Error()).To(ContainSubstring("Record does not exist")) }) @@ -114,7 +126,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { }, } - _, err := r.reconcileDNSRecord(context.TODO(), dnsRecord, zone) + _, err := r.reconcileDNSRecord(context.TODO(), dnsRecord, zone, cloudflareAPI) g.Expect(err).ToNot(HaveOccurred()) g.Expect(dnsRecord.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -127,7 +139,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { g.Expect(dnsRecord.Status.RecordID).To(Equal(cloudflareDNSRecord.ID)) g.Expect(cloudflareDNSRecord.Content).To(Equal(ip.Spec.Address)) - _ = r.reconcileDelete(context.TODO(), zone.Status.ID, dnsRecord) + _ = r.reconcileDelete(context.TODO(), cloudflareAPI, zone.Status.ID, dnsRecord) _, err = cloudflareAPI.GetDNSRecord(context.TODO(), cloudflare.ZoneIdentifier(zone.Status.ID), dnsRecord.Status.RecordID) g.Expect(err.Error()).To(ContainSubstring("Record does not exist")) }) @@ -150,7 +162,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { Proxied: cloudflareDNSRecord.Proxied, } - _, err = r.reconcileDNSRecord(context.TODO(), dnsRecord, zone) + _, err = r.reconcileDNSRecord(context.TODO(), dnsRecord, zone, cloudflareAPI) g.Expect(err).ToNot(HaveOccurred()) g.Expect(dnsRecord.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -159,7 +171,7 @@ func TestDNSRecordReconciler_reconcileDNSRecord(t *testing.T) { g.Expect(dnsRecord.Status.RecordID).To(Equal(cloudflareDNSRecord.ID)) - _ = r.reconcileDelete(context.TODO(), zone.Status.ID, dnsRecord) + _ = r.reconcileDelete(context.TODO(), cloudflareAPI, zone.Status.ID, dnsRecord) _, err = cloudflareAPI.GetDNSRecord(context.TODO(), cloudflare.ZoneIdentifier(zone.Status.ID), dnsRecord.Status.RecordID) g.Expect(err.Error()).To(ContainSubstring("Record does not exist")) }) diff --git a/internal/controller/zone_controller.go b/internal/controller/zone_controller.go index 9d3b87a0..0b730fe7 100644 --- a/internal/controller/zone_controller.go +++ b/internal/controller/zone_controller.go @@ -50,7 +50,7 @@ type ZoneReconciler struct { RetryInterval time.Duration - CloudflareAPI *cloudflare.API + AccountManager *AccountManager } var errWaitForZone = errors.New("must wait for zone") @@ -121,12 +121,21 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul // reconcileZone reconciles the zone func (r *ZoneReconciler) reconcileZone(ctx context.Context, zone *cloudflareoperatoriov1.Zone) (ctrl.Result, error) { - if r.CloudflareAPI.APIToken == "" { - intconditions.MarkUnknown(zone, "Cloudflare account is not ready") + cloudflareAPI, accountName, err := r.AccountManager.AccountForZone(zone.Spec.Name) + if errors.Is(err, errNoAccountForZone) { + intconditions.MarkFalse(zone, fmt.Errorf("no Cloudflare account manages zone %q", zone.Spec.Name)) + return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount + } + if errors.Is(err, errMultipleAccounts) { + intconditions.MarkFalse(zone, fmt.Errorf("multiple Cloudflare accounts manage zone %q: %s", zone.Spec.Name, accountName)) + return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount + } + if err != nil { + intconditions.MarkFalse(zone, err) return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForAccount } - zoneID, err := r.CloudflareAPI.ZoneIDByName(zone.Spec.Name) + zoneID, err := cloudflareAPI.ZoneIDByName(zone.Spec.Name) if err != nil { intconditions.MarkFalse(zone, err) return ctrl.Result{RequeueAfter: r.RetryInterval}, errWaitForZone @@ -135,7 +144,7 @@ func (r *ZoneReconciler) reconcileZone(ctx context.Context, zone *cloudflareoper zone.Status.ID = zoneID if zone.Spec.Prune { - if err := r.handlePrune(ctx, zone); err != nil { + if err := r.handlePrune(ctx, zone, cloudflareAPI); err != nil { intconditions.MarkFalse(zone, fmt.Errorf("failed to prune DNS records: %v", err)) return ctrl.Result{RequeueAfter: r.RetryInterval}, nil } @@ -147,7 +156,7 @@ func (r *ZoneReconciler) reconcileZone(ctx context.Context, zone *cloudflareoper } // handlePrune deletes DNS records that are not managed by the operator if enabled -func (r *ZoneReconciler) handlePrune(ctx context.Context, zone *cloudflareoperatoriov1.Zone) error { +func (r *ZoneReconciler) handlePrune(ctx context.Context, zone *cloudflareoperatoriov1.Zone, cloudflareAPI *cloudflare.API) error { log := ctrl.LoggerFrom(ctx) dnsRecords := &cloudflareoperatoriov1.DNSRecordList{} @@ -156,7 +165,7 @@ func (r *ZoneReconciler) handlePrune(ctx context.Context, zone *cloudflareoperat return client.IgnoreNotFound(err) } - cloudflareDNSRecords, _, err := r.CloudflareAPI.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{}) + cloudflareDNSRecords, _, err := cloudflareAPI.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflare.ListDNSRecordsParams{}) if err != nil { intconditions.MarkFalse(zone, err) return err @@ -174,7 +183,7 @@ func (r *ZoneReconciler) handlePrune(ctx context.Context, zone *cloudflareoperat } if _, found := dnsRecordMap[cloudflareDNSRecord.ID]; !found { - if err := r.CloudflareAPI.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflareDNSRecord.ID); err != nil && err.Error() != "Record does not exist. (81044)" { + if err := cloudflareAPI.DeleteDNSRecord(ctx, cloudflare.ZoneIdentifier(zone.Status.ID), cloudflareDNSRecord.ID); err != nil && err.Error() != "Record does not exist. (81044)" { return err } log.Info("Deleted DNS record on Cloudflare", "name", cloudflareDNSRecord.Name) diff --git a/internal/controller/zone_controller_test.go b/internal/controller/zone_controller_test.go index faf2c5f8..98c6bfb4 100644 --- a/internal/controller/zone_controller_test.go +++ b/internal/controller/zone_controller_test.go @@ -33,6 +33,14 @@ import ( ) func TestZoneReconciler_reconcileZone(t *testing.T) { + g := NewWithT(t) + + apiToken := os.Getenv("CF_API_TOKEN") + g.Expect(apiToken).ToNot(BeEmpty(), "CF_API_TOKEN must be set for this test") + + cloudflareAPI, err := cloudflare.NewWithAPIToken(apiToken) + g.Expect(err).ToNot(HaveOccurred()) + zone := &cloudflareoperatoriov1.Zone{ ObjectMeta: metav1.ObjectMeta{ Name: "zone", @@ -42,15 +50,20 @@ func TestZoneReconciler_reconcileZone(t *testing.T) { }, } + accountManager := NewAccountManager() + accountManager.UpsertAccount("account", cloudflareAPI, apiToken, []string{zone.Spec.Name}) + r := &ZoneReconciler{ Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithObjects(zone). Build(), - CloudflareAPI: &cloudflareAPI, + AccountManager: accountManager, } zoneID := os.Getenv("CF_ZONE_ID") + g.Expect(zoneID).ToNot(BeEmpty(), "CF_ZONE_ID must be set for this test") + var testRecord cloudflare.DNSRecord t.Run("create dns record for testing", func(t *testing.T) { @@ -134,6 +147,7 @@ func TestZoneReconciler_reconcileZone(t *testing.T) { g := NewWithT(t) zone.Spec.Name = "not-found.org" + accountManager.UpsertAccount("account", cloudflareAPI, apiToken, []string{zone.Spec.Name}) _, err := r.reconcileZone(context.TODO(), zone) g.Expect(err).To(HaveOccurred()) @@ -146,13 +160,13 @@ func TestZoneReconciler_reconcileZone(t *testing.T) { t.Run("reconcile zone error account not ready", func(t *testing.T) { g := NewWithT(t) - cloudflareAPI.APIToken = "" + accountManager.RemoveAccount("account") _, err := r.reconcileZone(context.TODO(), zone) g.Expect(err).To(Equal(errWaitForAccount)) g.Expect(zone.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ - *conditions.UnknownCondition(cloudflareoperatoriov1.ConditionTypeReady, cloudflareoperatoriov1.ConditionReasonNotReady, "Cloudflare account is not ready"), + *conditions.FalseCondition(cloudflareoperatoriov1.ConditionTypeReady, cloudflareoperatoriov1.ConditionReasonFailed, "no Cloudflare account manages zone \"not-found.org\""), })) }) }