OSAC-496: Remove subnet/tenant fallback dead code

internal/auth/default_tenancy_logic.go

@@ -75,15 +75,20 @@ func (p *DefaultTenancyLogic) DetermineAssignableTenants(ctx context.Context) (r
 }
 
 // DetermineDefaultTenant extracts the subject from the auth context and returns the tenant that will be assigned
-// by default to objects. When the subject has access to all tenants (e.g. an admin), the default is the shared
-// tenant because a universal set can't be stored as the tenant of an object.
+// by default to objects when the request does not specify one explicitly.
 func (p *DefaultTenancyLogic) DetermineDefaultTenant(ctx context.Context) (result string, err error) {
 	assignable, err := p.DetermineAssignableTenants(ctx)
 	if err != nil {
 		return
 	}
 	if !assignable.Finite() {
-		result = SharedTenant
+		subject := SubjectFromContext(ctx)
+		p.logger.ErrorContext(
+			ctx,
+			"Subject has access to all tenants but no explicit tenant was provided",
+			slog.String("user", subject.User),
+		)
+		err = ErrExplicitTenantRequired
 		return
 	}
 	inclusions := assignable.Inclusions()
@@ -100,7 +105,7 @@ func (p *DefaultTenancyLogic) DetermineVisibleTenants(ctx context.Context) (resu
 	subject := SubjectFromContext(ctx)
 	result = subject.Tenants
 	if result.Finite() {
-		result = SharedTenants.Union(result)
+		result = collections.NewSet(SharedTenant).Union(result)
 	}
 	return
 }

internal/auth/default_tenancy_logic_test.go

@@ -15,6 +15,7 @@ package auth
 
 import (
 	"context"
+	"errors"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -119,15 +120,15 @@ var _ = Describe("Default tenancy logic", func() {
 			Expect(result).To(BeElementOf("tenant-a", "tenant-b"))
 		})
 
-		It("Returns shared when tenants is universal", func() {
+		It("Fails when tenants is universal", func() {
 			subject := &Subject{
 				User:    "my_user",
 				Tenants: AllTenants,
 			}
 			ctx = ContextWithSubject(ctx, subject)
-			result, err := logic.DetermineDefaultTenant(ctx)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(result).To(Equal(SharedTenant))
+			_, err := logic.DetermineDefaultTenant(ctx)
+			Expect(err).To(MatchError(ErrExplicitTenantRequired))
+			Expect(errors.Is(err, ErrExplicitTenantRequired)).To(BeTrue())
 		})
 
 		It("Fails if the subject has an empty tenants set", func() {
@@ -151,7 +152,7 @@ var _ = Describe("Default tenancy logic", func() {
 			ctx = ContextWithSubject(ctx, subject)
 			result, err := logic.DetermineVisibleTenants(ctx)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(result.Equal(SharedTenants.Union(collections.NewSet("tenant-a", "tenant-b")))).To(BeTrue())
+			Expect(result.Equal(collections.NewSet(SharedTenant).Union(collections.NewSet("tenant-a", "tenant-b")))).To(BeTrue())
 		})
 
 		It("Returns universal set when tenants is universal", func() {
@@ -173,7 +174,7 @@ var _ = Describe("Default tenancy logic", func() {
 			ctx = ContextWithSubject(ctx, subject)
 			result, err := logic.DetermineVisibleTenants(ctx)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(result.Equal(SharedTenants)).To(BeTrue())
+			Expect(result.Equal(collections.NewSet(SharedTenant))).To(BeTrue())
 		})
 	})
 })

internal/auth/guest_tenancy_logic.go

@@ -67,7 +67,7 @@ func (p *GuestTenancyLogic) DetermineDefaultTenant(_ context.Context) (result st
 // DetermineVisibleTenants returns a set containing both the guest and shared tenants, allowing guest users to see
 // objects from both tenants.
 func (p *GuestTenancyLogic) DetermineVisibleTenants(_ context.Context) (result collections.Set[string], err error) {
-	result = GuestTenants.Union(SharedTenants)
+	result = GuestTenants.Union(collections.NewSet(SharedTenant))
 	return
 }
 

internal/auth/guest_tenancy_logic_test.go

@@ -18,6 +18,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/osac-project/fulfillment-service/internal/collections"
 )
 
 var _ = Describe("Guest tenancy logic", func() {
@@ -60,7 +61,7 @@ var _ = Describe("Guest tenancy logic", func() {
 		It("Should return the guest and shared tenants", func() {
 			result, err := logic.DetermineVisibleTenants(ctx)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(result.Equal(GuestTenants.Union(SharedTenants))).To(BeTrue())
+			Expect(result.Equal(GuestTenants.Union(collections.NewSet(SharedTenant)))).To(BeTrue())
 		})
 	})
 })

internal/auth/tenancy_logic.go

@@ -15,10 +15,17 @@ package auth
 
 import (
 	"context"
+	"errors"
 
 	"github.com/osac-project/fulfillment-service/internal/collections"
 )
 
+// ErrExplicitTenantRequired is returned when a subject with universal tenant access
+// creates or updates an object without specifying a tenant.
+var ErrExplicitTenantRequired = errors.New(
+	"explicit tenant is required when subject has access to all tenants",
+)
+
 // TenancyLogic defines the logic for determining object tenancy and access control.
 //
 //go:generate mockgen -destination=tenancy_logic_mock.go -package=auth . TenancyLogic
@@ -46,8 +53,5 @@ var SystemTenants = collections.NewSet(SystemTenant)
 // SharedTenant is the tenant that is always visible to all users.
 const SharedTenant = "shared"
 
-// SharedTenants is the set of tenants that are always visible to all users.
-var SharedTenants = collections.NewSet(SharedTenant)
-
 // AllTenants is the set of all tenants that are possible.
 var AllTenants = collections.NewUniversalSet[string]()

internal/cmd/cli/create/hub/create_hub_cmd.go

@@ -21,6 +21,7 @@ import (
 	"google.golang.org/protobuf/proto"
 
 	privatev1 "github.com/osac-project/fulfillment-service/internal/api/osac/private/v1"
+	"github.com/osac-project/fulfillment-service/internal/auth"
 	"github.com/osac-project/fulfillment-service/internal/config"
 	"github.com/osac-project/fulfillment-service/internal/terminal"
 )
@@ -54,6 +55,12 @@ func Cmd() *cobra.Command {
 		"",
 		namespaceFlagHelp,
 	)
+	flags.StringVar(
+		&runner.tenant,
+		"tenant",
+		auth.SharedTenant,
+		tenantFlagHelp,
+	)
 	return result
 }
 
@@ -62,6 +69,7 @@ type runnerContext struct {
 	id         string
 	kubeconfig string
 	namespace  string
+	tenant     string
 }
 
 func (c *runnerContext) run(cmd *cobra.Command, args []string) error {
@@ -87,8 +95,8 @@ func (c *runnerContext) run(cmd *cobra.Command, args []string) error {
 	if c.kubeconfig == "" {
 		return fmt.Errorf("kubeconfig file is required")
 	}
-	if c.namespace == "" {
-		return fmt.Errorf("namespace name is required")
+	if c.tenant == "" {
+		return fmt.Errorf("tenant is required")
 	}
 
 	// Create the gRPC connection from the configuration:
@@ -110,6 +118,9 @@ func (c *runnerContext) run(cmd *cobra.Command, args []string) error {
 	// Prepare the hub:
 	hub := privatev1.Hub_builder{
 		Id: c.id,
+		Metadata: privatev1.Metadata_builder{
+			Tenant: c.tenant,
+		}.Build(),
 		Spec: privatev1.HubSpec_builder{
 			Kubeconfig: kubeconfig,
 			Namespace:  c.namespace,
@@ -149,3 +160,7 @@ API.
 const namespaceFlagHelp = `
 _NAMESPACE_ - Namespace where cluster orders will be created.
 `
+
+const tenantFlagHelp = `
+_TENANT_ - Tenant that owns the hub. Defaults to the shared tenant.
+`

internal/servers/generic_server.go

@@ -1197,22 +1197,6 @@ func (s *GenericServer[O]) determineAssignedTenant(ctx context.Context,
 		return
 	}
 
-	// Determine the default tenant:
-	defaultTenant, err := s.tenancyLogic.DetermineDefaultTenant(ctx)
-	if err != nil {
-		s.logger.ErrorContext(
-			ctx,
-			"Failed to determine default tenant",
-			slog.Any("error", err),
-		)
-		err = grpcstatus.Errorf(grpccodes.Internal, "failed to determine default tenant")
-		return
-	}
-	if defaultTenant == "" {
-		err = grpcstatus.Errorf(grpccodes.PermissionDenied, "there is no default tenant")
-		return
-	}
-
 	// Get the tenant from the request and current object:
 	requestTenant := s.getTenant(requestObject)
 	currentTenant := s.getTenant(currentObject)
@@ -1249,12 +1233,32 @@ func (s *GenericServer[O]) determineAssignedTenant(ctx context.Context,
 		return
 	}
 
-	// Fall back to the current tenant or the default:
+	// Fall back to the current tenant when the request does not specify one:
 	if currentTenant != "" {
 		result = currentTenant
-	} else {
-		result = defaultTenant
+		return
+	}
+
+	// Determine the default tenant only when neither request nor current object specify one:
+	defaultTenant, err := s.tenancyLogic.DetermineDefaultTenant(ctx)
+	if err != nil {
+		if errors.Is(err, auth.ErrExplicitTenantRequired) {
+			err = grpcstatus.Error(grpccodes.PermissionDenied, err.Error())
+			return
+		}
+		s.logger.ErrorContext(
+			ctx,
+			"Failed to determine default tenant",
+			slog.Any("error", err),
+		)
+		err = grpcstatus.Errorf(grpccodes.Internal, "failed to determine default tenant")
+		return
+	}
+	if defaultTenant == "" {
+		err = grpcstatus.Errorf(grpccodes.PermissionDenied, "there is no default tenant")
+		return
 	}
+	result = defaultTenant
 	return
 }
 

internal/servers/servers_tenancy_test.go

@@ -300,6 +300,117 @@ var _ = Describe("Tenancy logic", func() {
 		Expect(status.Message()).To(Equal("tenant 'your-tenant' doesn't exist"))
 	})
 
+	It("Uses explicit tenant when subject has access to all tenants", func() {
+		// Create a tenancy logic that represents a universal admin:
+		tenancy := auth.NewMockTenancyLogic(ctrl)
+		tenancy.EXPECT().DetermineAssignableTenants(gomock.Any()).
+			Return(auth.AllTenants, nil).
+			AnyTimes()
+		tenancy.EXPECT().DetermineVisibleTenants(gomock.Any()).
+			Return(auth.AllTenants, nil).
+			AnyTimes()
+
+		// Create the template using the DAO:
+		templatesDao, err := dao.NewGenericDAO[*privatev1.ClusterTemplate]().
+			SetLogger(logger).
+			SetTenancyLogic(tenancy).
+			Build()
+		Expect(err).ToNot(HaveOccurred())
+		_, err = templatesDao.Create().
+			SetObject(
+				privatev1.ClusterTemplate_builder{
+					Id:          "my-template",
+					Title:       "My template",
+					Description: "My template",
+					Metadata: privatev1.Metadata_builder{
+						Tenant: "my-tenant",
+					}.Build(),
+				}.Build(),
+			).
+			Do(ctx)
+		Expect(err).ToNot(HaveOccurred())
+
+		// Create the clusters server:
+		clustersServer, err := NewClustersServer().
+			SetLogger(logger).
+			SetAttributionLogic(attribution).
+			SetTenancyLogic(tenancy).
+			SetScheme(testScheme).
+			Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		// Create a cluster with an explicit tenant and verify it succeeds without a default tenant:
+		response, err := clustersServer.Create(ctx, publicv1.ClustersCreateRequest_builder{
+			Object: publicv1.Cluster_builder{
+				Metadata: publicv1.Metadata_builder{
+					Tenant: "my-tenant",
+				}.Build(),
+				Spec: publicv1.ClusterSpec_builder{
+					Template: "my-template",
+				}.Build(),
+			}.Build(),
+		}.Build())
+		Expect(err).ToNot(HaveOccurred())
+
+		cluster := response.GetObject()
+		Expect(cluster).ToNot(BeNil())
+		Expect(cluster.GetMetadata().GetTenant()).To(Equal("my-tenant"))
+	})
+
+	It("Rejects create without tenant when subject has access to all tenants", func() {
+		tenancy := auth.NewMockTenancyLogic(ctrl)
+		tenancy.EXPECT().DetermineAssignableTenants(gomock.Any()).
+			Return(auth.AllTenants, nil).
+			AnyTimes()
+		tenancy.EXPECT().DetermineVisibleTenants(gomock.Any()).
+			Return(auth.AllTenants, nil).
+			AnyTimes()
+		tenancy.EXPECT().DetermineDefaultTenant(gomock.Any()).
+			Return("", auth.ErrExplicitTenantRequired).
+			Times(1)
+
+		templatesDao, err := dao.NewGenericDAO[*privatev1.ClusterTemplate]().
+			SetLogger(logger).
+			SetTenancyLogic(tenancy).
+			Build()
+		Expect(err).ToNot(HaveOccurred())
+		_, err = templatesDao.Create().
+			SetObject(
+				privatev1.ClusterTemplate_builder{
+					Id:          "my-template",
+					Title:       "My template",
+					Description: "My template",
+					Metadata: privatev1.Metadata_builder{
+						Tenant: "my-tenant",
+					}.Build(),
+				}.Build(),
+			).
+			Do(ctx)
+		Expect(err).ToNot(HaveOccurred())
+
+		clustersServer, err := NewClustersServer().
+			SetLogger(logger).
+			SetAttributionLogic(attribution).
+			SetTenancyLogic(tenancy).
+			SetScheme(testScheme).
+			Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		response, err := clustersServer.Create(ctx, publicv1.ClustersCreateRequest_builder{
+			Object: publicv1.Cluster_builder{
+				Spec: publicv1.ClusterSpec_builder{
+					Template: "my-template",
+				}.Build(),
+			}.Build(),
+		}.Build())
+		Expect(response).To(BeNil())
+		Expect(err).To(HaveOccurred())
+		status, ok := grpcstatus.FromError(err)
+		Expect(ok).To(BeTrue())
+		Expect(status.Code()).To(Equal(grpccodes.PermissionDenied))
+		Expect(status.Message()).To(Equal(auth.ErrExplicitTenantRequired.Error()))
+	})
+
 	It("Rejects object creation when tenant is visible to the user, but doesn't exist in the database", func() {
 		// Create a tenancy logic that returns visible tenants:
 		tenancy := auth.NewMockTenancyLogic(ctrl)

it/it_annotations_test.go

@@ -46,7 +46,8 @@ var _ = Describe("Annotations", func() {
 		hostTypeId = fmt.Sprintf("my-host-type-%s", uuid.New())
 		_, err := hostTypesClient.Create(ctx, privatev1.HostTypesCreateRequest_builder{
 			Object: privatev1.HostType_builder{
-				Id: hostTypeId,
+				Metadata: sharedMetadata(),
+				Id:       hostTypeId,
 			}.Build(),
 		}.Build())
 		Expect(err).ToNot(HaveOccurred())
@@ -60,6 +61,7 @@ var _ = Describe("Annotations", func() {
 		templateId = fmt.Sprintf("my-template-%s", uuid.New())
 		_, err = templatesClient.Create(ctx, privatev1.ClusterTemplatesCreateRequest_builder{
 			Object: privatev1.ClusterTemplate_builder{
+				Metadata:    sharedMetadata(),
 				Id:          templateId,
 				Title:       "My template %s",
 				Description: "My template.",