diff --git a/internal/kube/site/site.go b/internal/kube/site/site.go index 37be16847..3a9afa9b9 100644 --- a/internal/kube/site/site.go +++ b/internal/kube/site/site.go @@ -228,7 +228,9 @@ func (s *Site) reconcile(siteDef *skupperv2alpha1.Site, inRecovery bool) error { s.currentGroups = s.groups() s.bindings.init(s, routerConfig) s.setBindingsConfiguredStatus(nil) - s.checkSecuredAccess() + if err := s.checkSecuredAccess(); err != nil { + return err + } } else if len(s.currentGroups) != len(s.groups()) { s.logger.Info("EnableHA setting changed for site", slog.String("namespace", siteDef.Namespace), @@ -1537,6 +1539,7 @@ func asSecuredAccessSpec(routerAccess *skupperv2alpha1.RouterAccess, group strin func (s *Site) checkSecuredAccess() error { groups := s.groups() + var errs []error for i, group := range groups { for _, la := range s.linkAccess { name := la.Name @@ -1548,15 +1551,18 @@ func (s *Site) checkSecuredAccess() error { "internal.skupper.io/routeraccess": la.Name, } if err := s.access.Ensure(s.namespace, name, asSecuredAccessSpec(la, group, s.site.DefaultIssuer()), annotations, routerAccessOwner(la)); err != nil { - //TODO: add message to site status s.logger.Error("Error ensuring SecuredAccess for RouterAccess", slog.String("key", la.Key()), slog.Any("error", err)) + errs = append(errs, fmt.Errorf("%s: %w", la.Key(), err)) } else { s.accessMapping[name] = newSecuredAccessMapping(la.Name, group) } } } + if len(errs) > 0 { + return stderrors.Join(errs...) + } return nil } diff --git a/internal/kube/site/site_test.go b/internal/kube/site/site_test.go index 6e804b05b..24a2b9d87 100644 --- a/internal/kube/site/site_test.go +++ b/internal/kube/site/site_test.go @@ -1112,6 +1112,72 @@ func TestSite_CheckRouterAccess(t *testing.T) { } } +func TestSite_checkSecuredAccess(t *testing.T) { + tests := []struct { + name string + skupperErrorMessage string + wantErr bool + wantAccessMapping int + }{ + { + name: "secured access ensured for each router access", + wantErr: false, + wantAccessMapping: 1, + }, + { + name: "secured access ensure failure is returned, not just logged", + skupperErrorMessage: "secured access create failed", + wantErr: true, + wantAccessMapping: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, err := newSiteMocks("test", nil, nil, tt.skupperErrorMessage, true) + assert.Assert(t, err) + + s.linkAccess["my-ra"] = &skupperv2alpha1.RouterAccess{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-ra", + Namespace: "test", + }, + Spec: skupperv2alpha1.RouterAccessSpec{ + AccessType: "loadbalancer", + }, + } + + err = s.checkSecuredAccess() + if (err != nil) != tt.wantErr { + t.Errorf("Site.checkSecuredAccess() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.skupperErrorMessage != "" && err != nil { + assert.ErrorContains(t, err, tt.skupperErrorMessage) + } + assert.Equal(t, tt.wantAccessMapping, len(s.accessMapping)) + }) + } +} + +func TestSite_checkSecuredAccess_reportsErrorOnInitialization(t *testing.T) { + s, err := newSiteMocks("test", nil, nil, "secured access create failed", true) + assert.Assert(t, err) + + s.linkAccess["my-ra"] = &skupperv2alpha1.RouterAccess{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-ra", + Namespace: "test", + }, + Spec: skupperv2alpha1.RouterAccessSpec{ + AccessType: "loadbalancer", + }, + } + + err = s.Reconcile(s.site) + assert.Assert(t, err != nil) + assert.Assert(t, !s.site.IsConfigured(), "site should not be reported as configured when SecuredAccess creation fails") +} + func Test_NetworkStatusUpdate(t *testing.T) { type args struct { siteRecord []skupperv2alpha1.SiteRecord