Merge branch 'main' into main

pkg/spireapi/entryapi.go

@@ -18,17 +18,34 @@ package spireapi
 
 import (
 	"context"
+	"fmt"
 
 	entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
+	"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
 	apitypes "github.com/spiffe/spire-api-sdk/proto/spire/api/types"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+const (
+	AdminField         Field = "admin"
+	DNSNamesField      Field = "dnsNames"
+	DownstreamField    Field = "downstream"
+	FederatesWithField Field = "federatesWith"
+	HintField          Field = "hint"
+	JWTSVIDTTLField    Field = "jwtSVIDTTL"
+	X509SVIDTTL        Field = "x509SVIDTTL"
+)
+
+type Field string
+
 type EntryClient interface {
 	ListEntries(ctx context.Context) ([]Entry, error)
 	CreateEntries(ctx context.Context, entries []Entry) ([]Status, error)
 	UpdateEntries(ctx context.Context, entries []Entry) ([]Status, error)
 	DeleteEntries(ctx context.Context, entryIDs []string) ([]Status, error)
+	GetUnsupportedFields(ctx context.Context, td string) (map[Field]struct{}, error)
 }
 
 func NewEntryClient(conn grpc.ClientConnInterface) EntryClient {
@@ -59,6 +76,65 @@ func (c entryClient) ListEntries(ctx context.Context) ([]Entry, error) {
 	return entriesFromAPI(entries)
 }
 
+func (c entryClient) GetUnsupportedFields(ctx context.Context, td string) (map[Field]struct{}, error) {
+	resp, err := c.api.BatchCreateEntry(ctx, &entryv1.BatchCreateEntryRequest{
+		Entries: []*apitypes.Entry{
+			{
+				ParentId: &types.SPIFFEID{
+					TrustDomain: td,
+					Path:        "/spire-controller-manager/unsupported-fields-test",
+				},
+				SpiffeId: &types.SPIFFEID{
+					TrustDomain: td,
+					Path:        "/spire-controller-manager/unsupported-fields-test",
+				},
+				Selectors: []*types.Selector{
+					{
+						Type:  "a",
+						Value: "1",
+					},
+				},
+				X509SvidTtl: 60,
+				JwtSvidTtl:  60,
+				Hint:        "hint",
+			},
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if len(resp.Results) != 1 {
+		return nil, fmt.Errorf("only one response expected but got %v", len(resp.Results))
+	}
+
+	result := resp.Results[0]
+	if result.Status.Code != int32(codes.OK) {
+		return nil, fmt.Errorf("failed to create entry: %v", result.Status.Message)
+	}
+
+	_, err = c.api.BatchDeleteEntry(ctx, &entryv1.BatchDeleteEntryRequest{
+		Ids: []string{
+			result.Entry.Id,
+		},
+	})
+	if err != nil {
+		log := log.FromContext(ctx)
+		log.Error(err, "failed to delete dummy entry", "entry_id", result.Entry.Id)
+	}
+
+	unsupportedFields := make(map[Field]struct{})
+	if result.Entry.JwtSvidTtl == 0 {
+		unsupportedFields[JWTSVIDTTLField] = struct{}{}
+	}
+
+	if result.Entry.Hint == "" {
+		unsupportedFields[HintField] = struct{}{}
+	}
+
+	return unsupportedFields, nil
+}
+
 func (c entryClient) CreateEntries(ctx context.Context, entries []Entry) ([]Status, error) {
 	statuses := make([]Status, 0, len(entries))
 	err := runBatch(len(entries), entryCreateBatchSize, func(start, end int) error {

pkg/spireapi/entryapi_test.go

@@ -157,6 +157,64 @@ func TestCreateEntries(t *testing.T) {
 	}
 }
 
+func TestGetUnsupportedFields(t *testing.T) {
+	for _, tc := range []struct {
+		desc                   string
+		cleanUnsupportedFields bool
+		createEntryErr         error
+		deleteEntryErr         error
+
+		expectErr    error
+		expectFields map[Field]struct{}
+	}{
+		{
+			desc:         "no old fields",
+			expectFields: make(map[Field]struct{}),
+		},
+		{
+			desc:                   "old fields found",
+			cleanUnsupportedFields: true,
+			expectFields: map[Field]struct{}{
+				HintField:       {},
+				JWTSVIDTTLField: {},
+			},
+		},
+		{
+			desc:           "failed to create entry",
+			createEntryErr: status.Error(codes.Internal, "oh no"),
+			expectErr:      status.Error(codes.Internal, "oh no"),
+		},
+		{
+			desc:                   "failed to delete entry",
+			cleanUnsupportedFields: true,
+			deleteEntryErr:         status.Error(codes.Internal, "oh no"),
+			expectFields: map[Field]struct{}{
+				HintField:       {},
+				JWTSVIDTTLField: {},
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			server, client := startEntryAPIServer(t)
+
+			server.batchCreateEntriesErr = tc.createEntryErr
+			server.batchDeleteEntriesErr = tc.deleteEntryErr
+			server.clearUnsupportedFields = tc.cleanUnsupportedFields
+
+			resp, err := client.GetUnsupportedFields(ctx, "domain.test")
+			if tc.expectErr != nil {
+				assertErrorIs(t, err, tc.expectErr)
+				assert.Empty(t, resp)
+
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectFields, resp)
+		})
+	}
+}
+
 func TestUpdateEntries(t *testing.T) {
 	server, client := startEntryAPIServer(t)
 
@@ -309,6 +367,8 @@ type entryServer struct {
 	mtx     sync.RWMutex
 	entries []*apitypes.Entry
 
+	clearUnsupportedFields bool
+
 	listEntriesErr        error
 	batchCreateEntriesErr error
 	batchUpdateEntriesErr error
@@ -336,6 +396,12 @@ func (s *entryServer) BatchCreateEntry(ctx context.Context, req *entryv1.BatchCr
 	resp := new(entryv1.BatchCreateEntryResponse)
 
 	for _, entry := range req.Entries {
+		//  Remove values from new fields to emulate an old server is used
+		if s.clearUnsupportedFields {
+			entry.JwtSvidTtl = 0
+			entry.Hint = ""
+		}
+
 		st := status.Convert(s.createEntry(entry))
 		result := &entryv1.BatchCreateEntryResponse_Result{
 			Status: &apitypes.Status{

pkg/spireentry/reconciler.go

@@ -23,8 +23,10 @@ import (
 	"io"
 	"regexp"
 	"sort"
+	"strings"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	spirev1alpha1 "github.com/spiffe/spire-controller-manager/api/v1alpha1"
 	"github.com/spiffe/spire-controller-manager/pkg/k8sapi"
@@ -67,11 +69,19 @@ func Reconciler(config ReconcilerConfig) reconciler.Reconciler {
 
 type entryReconciler struct {
 	config ReconcilerConfig
+
+	unsupportedFields        map[spireapi.Field]struct{}
+	nextGetUnsupportedFields time.Time
 }
 
 func (r *entryReconciler) reconcile(ctx context.Context) {
 	log := log.FromContext(ctx)
 
+	if time.Now().After(r.nextGetUnsupportedFields) {
+		r.recalculateUnsupportFields(ctx, log)
+	}
+	unsupportedFields := r.unsupportedFields
+
 	// Load current entries from SPIRE server.
 	currentEntries, err := r.listEntries(ctx)
 	if err != nil {
@@ -125,7 +135,7 @@ func (r *entryReconciler) reconcile(ctx context.Context) {
 				toCreate = append(toCreate, preferredEntry)
 			} else {
 				preferredEntry.Entry.ID = s.Current[0].ID
-				if outdatedFields := getOutdatedEntryFields(preferredEntry.Entry, s.Current[0]); len(outdatedFields) != 0 {
+				if outdatedFields := getOutdatedEntryFields(preferredEntry.Entry, s.Current[0], unsupportedFields); len(outdatedFields) != 0 {
 					// Current field does not match. Nothing to do.
 					toUpdate = append(toUpdate, preferredEntry)
 				}
@@ -179,11 +189,48 @@ func (r *entryReconciler) reconcile(ctx context.Context) {
 	}
 }
 
+func (r *entryReconciler) recalculateUnsupportFields(ctx context.Context, log logr.Logger) {
+	unsupportedFields, err := r.getUnsupportedFields(ctx)
+	if err != nil {
+		log.Error(err, "failed to get unsupported fields")
+		return
+	}
+
+	// Get the list of new fields that are marked as unsupported
+	var newUnsupportedFields []string
+	for key := range unsupportedFields {
+		if _, ok := r.unsupportedFields[key]; !ok {
+			newUnsupportedFields = append(newUnsupportedFields, string(key))
+		}
+	}
+	if len(newUnsupportedFields) > 0 {
+		log.Info("New unsupported fields in SPIRE server found", "fields", strings.Join(newUnsupportedFields, ","))
+	}
+
+	// Get the list of fields that used to be unsupported but now are supported
+	var supportedFields []string
+	for key := range r.unsupportedFields {
+		if _, ok := unsupportedFields[key]; !ok {
+			supportedFields = append(supportedFields, string(key))
+		}
+	}
+	if len(supportedFields) > 0 {
+		log.Info("Fields previously unsupported are now supported on SPIRE server", "fields", strings.Join(supportedFields, ","))
+	}
+
+	r.unsupportedFields = unsupportedFields
+	r.nextGetUnsupportedFields = time.Now().Add(10 * time.Minute)
+}
+
 func (r *entryReconciler) listEntries(ctx context.Context) ([]spireapi.Entry, error) {
 	// TODO: cache?
 	return r.config.EntryClient.ListEntries(ctx)
 }
 
+func (r *entryReconciler) getUnsupportedFields(ctx context.Context) (map[spireapi.Field]struct{}, error) {
+	return r.config.EntryClient.GetUnsupportedFields(ctx, r.config.TrustDomain.Name())
+}
+
 func (r *entryReconciler) listClusterStaticEntries(ctx context.Context) ([]*ClusterStaticEntry, error) {
 	clusterStaticEntries, err := k8sapi.ListClusterStaticEntries(ctx, r.config.K8sClient)
 	if err != nil {
@@ -477,31 +524,35 @@ func objectCmp(a, b byObject) int {
 	}
 }
 
-func getOutdatedEntryFields(newEntry, oldEntry spireapi.Entry) []string {
+func getOutdatedEntryFields(newEntry, oldEntry spireapi.Entry, unsupportedFields map[spireapi.Field]struct{}) []spireapi.Field {
 	// We don't need to bother with the parent ID, the SPIFFE ID, or the
 	// selectors since they are part of the uniqueness check that resulted in
 	// the AlreadyExists error code.
-	var outdated []string
+	var outdated []spireapi.Field
 	if oldEntry.X509SVIDTTL != newEntry.X509SVIDTTL {
-		outdated = append(outdated, "x509SVIDTTL")
+		outdated = append(outdated, spireapi.X509SVIDTTL)
 	}
 	if oldEntry.JWTSVIDTTL != newEntry.JWTSVIDTTL {
-		outdated = append(outdated, "jwtSVIDTTL")
+		if _, ok := unsupportedFields[spireapi.JWTSVIDTTLField]; !ok {
+			outdated = append(outdated, spireapi.JWTSVIDTTLField)
+		}
 	}
 	if !trustDomainsMatch(oldEntry.FederatesWith, newEntry.FederatesWith) {
-		outdated = append(outdated, "federatesWith")
+		outdated = append(outdated, spireapi.FederatesWithField)
 	}
 	if oldEntry.Admin != newEntry.Admin {
-		outdated = append(outdated, "admin")
+		outdated = append(outdated, spireapi.AdminField)
 	}
 	if oldEntry.Downstream != newEntry.Downstream {
-		outdated = append(outdated, "downstream")
+		outdated = append(outdated, spireapi.DownstreamField)
 	}
 	if !stringsMatch(oldEntry.DNSNames, newEntry.DNSNames) {
-		outdated = append(outdated, "dnsNames")
+		outdated = append(outdated, spireapi.DNSNamesField)
 	}
 	if oldEntry.Hint != newEntry.Hint {
-		outdated = append(outdated, "hint")
+		if _, ok := unsupportedFields[spireapi.HintField]; !ok {
+			outdated = append(outdated, spireapi.HintField)
+		}
 	}
 
 	return outdated