diff --git a/rest-api/api/pkg/api/handler/instance.go b/rest-api/api/pkg/api/handler/instance.go index c741c285af..286b60e9c4 100644 --- a/rest-api/api/pkg/api/handler/instance.go +++ b/rest-api/api/pkg/api/handler/instance.go @@ -180,11 +180,24 @@ func NewCreateInstanceHandler(dbSession *cdb.Session, tc temporalClient.Client, } } +// canTenantUseOperatingSystem reports whether an Operating System can be used +// by a Tenant. Tenant-owned definitions are private to their owner. Provider- +// owned Templated iPXE definitions are shared with Tenants through their +// synchronized Site associations, which are validated separately before the +// definition is sent to Core. +func canTenantUseOperatingSystem(os *cdbm.OperatingSystem, tenantID string) bool { + if os.TenantID != nil { + return os.TenantID.String() == tenantID + } + + return os.InfrastructureProviderID != nil && os.Type == cdbm.OperatingSystemTypeTemplatedIPXE +} + // validateTemplatedIpxeOsForSite guards the Templated iPXE Operating System // selection paths (Instance create / update / batch-create) before the OS ID is -// sent to Core. Caller authorization and tenant/OS ownership are already enforced +// sent to Core. Caller authorization and tenant/OS access are already enforced // by the handlers (ValidateOrgMembership / ValidateUserRoles) and the per-request -// ownership check, so this enforces the site-availability contract specific to +// usability check, so this enforces the site-availability contract specific to // templated OSes: the OS definition must be synchronized to the Instance's Site // (a Synced OperatingSystemSiteAssociation) so the Site can render the template // at provisioning time. @@ -281,9 +294,10 @@ func (cih CreateInstanceHandler) buildInstanceCreateRequestOsConfig(c echo.Conte return c.Str("OperatingSystem ID", os.ID.String()) }) - // Confirm ownership between tenant and OS. - if os.TenantID.String() != apiRequest.TenantID { - logger.Error().Msg("OperatingSystem in request is not owned by tenant") + // Confirm the Tenant can use the OS. Provider-owned Templated iPXE OSes are + // shared through synchronized Site associations validated below. + if !canTenantUseOperatingSystem(os, apiRequest.TenantID) { + logger.Error().Msg("OperatingSystem in request is not usable by tenant") return nil, nil, cutil.NewAPIError(http.StatusBadRequest, "OperatingSystem specified in request is not owned by Tenant", nil) } @@ -2232,9 +2246,10 @@ func (uih UpdateInstanceHandler) buildInstanceUpdateRequestOsConfig(c echo.Conte return c.Str("OperatingSystem ID", os.ID.String()) }) - // Confirm ownership between tenant and OS. - if os.TenantID.String() != instance.Tenant.ID.String() { - logger.Error().Msg("OperatingSystem in request is not owned by tenant") + // Confirm the Tenant can use the OS. Provider-owned Templated iPXE OSes + // are shared through synchronized Site associations validated below. + if !canTenantUseOperatingSystem(os, instance.Tenant.ID.String()) { + logger.Error().Msg("OperatingSystem in request is not usable by tenant") return nil, nil, cutil.NewAPIError(http.StatusBadRequest, "Operating system specified in request is not owned by Tenant", nil) } diff --git a/rest-api/api/pkg/api/handler/instance_test.go b/rest-api/api/pkg/api/handler/instance_test.go index 69acbebe64..eeb5e4efa1 100644 --- a/rest-api/api/pkg/api/handler/instance_test.go +++ b/rest-api/api/pkg/api/handler/instance_test.go @@ -10618,6 +10618,20 @@ func TestBuildInstanceOsConfig_TemplatedIPXE(t *testing.T) { osSynced := buildOS("tmpl-os-instance-os-synced") testInstanceBuildOperatingSystemSiteAssociation(t, dbSession, site.ID, osSynced.ID) + providerOS := &cdbm.OperatingSystem{ + ID: uuid.New(), + Name: "tmpl-os-instance-provider-os", + Org: ip.Org, + InfrastructureProviderID: &ip.ID, + Type: cdbm.OperatingSystemTypeTemplatedIPXE, + IsActive: true, + Status: cdbm.OperatingSystemStatusReady, + CreatedBy: user.ID, + } + _, err := dbSession.DB.NewInsert().Model(providerOS).Exec(context.Background()) + require.NoError(t, err) + testInstanceBuildOperatingSystemSiteAssociation(t, dbSession, site.ID, providerOS.ID) + t.Run("create", func(t *testing.T) { ec := newTemplatedOsEchoContext(t) h := CreateInstanceHandler{dbSession: dbSession, cfg: cfg} @@ -10669,6 +10683,52 @@ func TestBuildInstanceOsConfig_TemplatedIPXE(t *testing.T) { assertTemplatedOsConfig(t, osConfig, osSynced.ID) }) + providerCases := []struct { + name string + build func() (*corev1.InstanceOperatingSystemConfig, *uuid.UUID, *cutil.APIError) + }{ + { + name: "create allows provider-owned OS", + build: func() (*corev1.InstanceOperatingSystemConfig, *uuid.UUID, *cutil.APIError) { + h := CreateInstanceHandler{dbSession: dbSession, cfg: cfg} + req := &model.APIInstanceCreateRequest{ + TenantID: tenant.ID.String(), + OperatingSystemID: cutil.GetPtr(providerOS.ID.String()), + } + return h.buildInstanceCreateRequestOsConfig(newTemplatedOsEchoContext(t), &logger, req, site) + }, + }, + { + name: "update allows provider-owned OS", + build: func() (*corev1.InstanceOperatingSystemConfig, *uuid.UUID, *cutil.APIError) { + h := UpdateInstanceHandler{dbSession: dbSession, cfg: cfg} + instance := &cdbm.Instance{ID: uuid.New(), TenantID: tenant.ID, Tenant: tenant} + req := &model.APIInstanceUpdateRequest{OperatingSystemID: cutil.GetPtr(providerOS.ID.String())} + return h.buildInstanceUpdateRequestOsConfig(newTemplatedOsEchoContext(t), &logger, req, instance, site) + }, + }, + { + name: "batch create allows provider-owned OS", + build: func() (*corev1.InstanceOperatingSystemConfig, *uuid.UUID, *cutil.APIError) { + h := BatchCreateInstanceHandler{dbSession: dbSession, cfg: cfg} + req := &model.APIBatchInstanceCreateRequest{ + TenantID: tenant.ID.String(), + OperatingSystemID: cutil.GetPtr(providerOS.ID.String()), + } + return h.buildBatchInstanceCreateRequestOsConfig(newTemplatedOsEchoContext(t), &logger, req, site) + }, + }, + } + for _, tc := range providerCases { + t.Run(tc.name, func(t *testing.T) { + osConfig, osID, apiErr := tc.build() + require.Nil(t, apiErr) + require.NotNil(t, osID) + assert.Equal(t, providerOS.ID, *osID) + assertTemplatedOsConfig(t, osConfig, providerOS.ID) + }) + } + // The following cases exercise the shared validator through the create path; // the same validator gates the update and batch paths. diff --git a/rest-api/api/pkg/api/handler/instancebatch.go b/rest-api/api/pkg/api/handler/instancebatch.go index c913c26c4b..e7734923f6 100644 --- a/rest-api/api/pkg/api/handler/instancebatch.go +++ b/rest-api/api/pkg/api/handler/instancebatch.go @@ -123,9 +123,10 @@ func (bcih BatchCreateInstanceHandler) buildBatchInstanceCreateRequestOsConfig(c return c.Str("OperatingSystem ID", os.ID.String()) }) - // Confirm ownership between tenant and OS. - if os.TenantID.String() != apiRequest.TenantID { - logger.Error().Msg("OperatingSystem in request is not owned by tenant") + // Confirm the Tenant can use the OS. Provider-owned Templated iPXE OSes are + // shared through synchronized Site associations validated below. + if !canTenantUseOperatingSystem(os, apiRequest.TenantID) { + logger.Error().Msg("OperatingSystem in request is not usable by tenant") return nil, nil, cutil.NewAPIError(http.StatusBadRequest, "OperatingSystem specified in request is not owned by Tenant", nil) }