Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

fix(#674): introduce nsUsername used as base-name for namespace names #675

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions controller/tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,22 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error {
return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err))
}

nsBaseName, err := tenant.ConstructNsBaseName(c.tenantService, env.RetrieveUserName(user.OpenShiftUsername))
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
"os_username": user.OpenShiftUsername,
}, "unable to construct namespace base name")
return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err))
}

// create openshift config
openshiftConfig := openshift.NewConfig(c.config, user.UserData, cluster.User, cluster.Token, cluster.APIURL)
tenant := &tenant.Tenant{
ID: ttoken.Subject(),
Email: ttoken.Email(),
OSUsername: user.OpenShiftUsername,
NsBaseName: nsBaseName,
}
err = c.tenantService.CreateTenant(tenant)
if err != nil {
Expand All @@ -104,8 +114,9 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error {
err = openshift.RawInitTenant(
ctx,
openshiftConfig,
InitTenant(ctx, openshiftConfig.MasterURL, c.tenantService, t, user.OpenShiftUsername),
InitTenant(ctx, openshiftConfig.MasterURL, c.tenantService, t),
user.OpenShiftUsername,
nsBaseName,
user.OpenShiftUserToken)

if err != nil {
Expand Down Expand Up @@ -156,7 +167,9 @@ func (c *TenantController) Update(ctx *app.UpdateTenantContext) error {

// update tenant config
tenant.OSUsername = user.OpenShiftUsername

if tenant.NsBaseName == "" {
tenant.NsBaseName = env.RetrieveUserName(user.OpenShiftUsername)
}
if err = c.tenantService.SaveTenant(tenant); err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
Expand All @@ -170,8 +183,9 @@ func (c *TenantController) Update(ctx *app.UpdateTenantContext) error {
err = openshift.RawUpdateTenant(
ctx,
openshiftConfig,
InitTenant(ctx, openshiftConfig.MasterURL, c.tenantService, t, user.OpenShiftUsername),
user.OpenShiftUsername)
InitTenant(ctx, openshiftConfig.MasterURL, c.tenantService, t),
user.OpenShiftUsername,
tenant.NsBaseName)

if err != nil {
sentry.LogError(ctx, map[string]interface{}{
Expand All @@ -198,6 +212,11 @@ func (c *TenantController) Clean(ctx *app.CleanTenantContext) error {
return jsonapi.JSONErrorResponse(ctx, err)
}

tenant, err := c.tenantService.GetTenant(ttoken.Subject())
if err != nil {
return jsonapi.JSONErrorResponse(ctx, errors.NewNotFoundError("tenants", ttoken.Subject().String()))
}

// restrict deprovision from cluster to internal users only
removeFromCluster := false
if user.UserData.FeatureLevel != nil && *user.UserData.FeatureLevel == "internal" {
Expand All @@ -216,7 +235,12 @@ func (c *TenantController) Clean(ctx *app.CleanTenantContext) error {
// create openshift config
openshiftConfig := openshift.NewConfig(c.config, user.UserData, cluster.User, cluster.Token, cluster.APIURL)

err = openshift.CleanTenant(ctx, openshiftConfig, user.OpenShiftUsername, removeFromCluster)
nsBaseName := tenant.NsBaseName
if nsBaseName == "" {
nsBaseName = env.RetrieveUserName(user.OpenShiftUsername)
}

err = openshift.CleanTenant(ctx, openshiftConfig, user.OpenShiftUsername, nsBaseName, removeFromCluster)
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}
Expand Down Expand Up @@ -253,10 +277,9 @@ func (c *TenantController) Show(ctx *app.ShowTenantContext) error {
}

// InitTenant is a Callback that assumes a new tenant is being created
func InitTenant(ctx context.Context, masterURL string, service tenant.Service, currentTenant *tenant.Tenant, openshiftUsername string) openshift.Callback {
func InitTenant(ctx context.Context, masterURL string, service tenant.Service, currentTenant *tenant.Tenant) openshift.Callback {
var maxResourceQuotaStatusCheck int32 = 50 // technically a global retry count across all ResourceQuota on all Tenant Namespaces
var currentResourceQuotaStatusCheck int32 // default is 0
username := env.RetrieveUserName(openshiftUsername)
return func(statusCode int, method string, request, response map[interface{}]interface{}) (string, map[interface{}]interface{}) {
log.Info(ctx, map[string]interface{}{
"status": statusCode,
Expand Down Expand Up @@ -290,7 +313,7 @@ func InitTenant(ctx context.Context, masterURL string, service tenant.Service, c
Name: name,
State: "created",
Version: env.GetLabelVersion(request),
Type: tenant.GetNamespaceType(name, username),
Type: tenant.GetNamespaceType(name, currentTenant.NsBaseName),
MasterURL: masterURL,
})

Expand All @@ -305,7 +328,7 @@ func InitTenant(ctx context.Context, masterURL string, service tenant.Service, c
Name: name,
State: "created",
Version: env.GetLabelVersion(request),
Type: tenant.GetNamespaceType(name, username),
Type: tenant.GetNamespaceType(name, currentTenant.NsBaseName),
MasterURL: masterURL,
})
} else if env.GetKind(request) == env.ValKindResourceQuota {
Expand Down
10 changes: 4 additions & 6 deletions environment/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,11 @@ func (t *Template) ReplaceVars(variables map[string]string) (string, error) {
})), nil
}

func CollectVars(user, masterUser string, config *configuration.Data) map[string]string {
userName := RetrieveUserName(user)

func CollectVars(osUsername, nsBaseName, masterUser string, config *configuration.Data) map[string]string {
vars := map[string]string{
varUserName: userName,
varProjectUser: user,
varProjectRequestingUser: user,
varUserName: nsBaseName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on how this is used but the naming looks odd.. username != basename

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind.. {USER_NAME}-che

varProjectUser: osUsername,
varProjectRequestingUser: osUsername,
varProjectAdminUser: masterUser,
}

Expand Down
4 changes: 2 additions & 2 deletions environment/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func TestSort(t *testing.T) {
defer reset()

template := environment.Template{Content: sortTemplate}
objects, err := template.Process(environment.CollectVars("developer", "master", data))
objects, err := template.Process(environment.CollectVars("developer", "developer", "master", data))

// when
sort.Sort(environment.ByKind(objects))
Expand All @@ -180,7 +180,7 @@ func TestParseNamespace(t *testing.T) {
template := environment.Template{Content: parseNamespaceTemplate}

// when
objects, err := template.Process(environment.CollectVars("developer", "master", data))
objects, err := template.Process(environment.CollectVars("developer", "developer", "master", data))

// then
require.NoError(t, err)
Expand Down
1 change: 1 addition & 0 deletions migration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func getMigrations() migrations {
m = append(m, steps{executeSQLFile("003-profiles.sql")})
m = append(m, steps{executeSQLFile("004-index-tenants-search.sql")})
m = append(m, steps{executeSQLFile("005-add-username-column-to-tenant.sql")})
m = append(m, steps{executeSQLFile("006-add-ns-base-name-column-to-tenant.sql")})

// Version N
//
Expand Down
3 changes: 3 additions & 0 deletions migration/sql-files/006-add-ns-base-name-column-to-tenant.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE tenants ADD COLUMN ns_base_name TEXT;
CREATE INDEX idx_ns_base_name ON tenants USING btree (ns_base_name);
UPDATE tenants SET ns_base_name = (SELECT name FROM namespaces n WHERE tenants.id = n.tenant_id AND n.type = 'user');
2 changes: 1 addition & 1 deletion openshift/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestDeleteAndGet(t *testing.T) {

func prepareObjectsAndOpts(t *testing.T, content string, config *configuration.Data) (environment.Objects, openshift.ApplyOptions) {
template := environment.Template{Content: content}
objects, err := template.Process(environment.CollectVars("aslak", "master", config))
objects, err := template.Process(environment.CollectVars("aslak", "aslak", "master", config))
require.NoError(t, err)
opts := openshift.ApplyOptions{
Config: openshift.Config{
Expand Down
4 changes: 2 additions & 2 deletions openshift/clean_tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
)

// CleanTenant clean or remove
func CleanTenant(ctx context.Context, config Config, username string, remove bool) error {
templs, err := LoadProcessedTemplates(ctx, config, username)
func CleanTenant(ctx context.Context, config Config, osUsername, nsBaseName string, remove bool) error {
templs, err := LoadProcessedTemplates(ctx, config, osUsername, nsBaseName)
if err != nil {
return err
}
Expand Down
11 changes: 5 additions & 6 deletions openshift/init_tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"gopkg.in/yaml.v2"
)

func RawInitTenant(ctx context.Context, config Config, callback Callback, openshiftUsername, usertoken string) error {
templs, err := LoadProcessedTemplates(ctx, config, openshiftUsername)
func RawInitTenant(ctx context.Context, config Config, callback Callback, openshiftUsername, nsBaseName, usertoken string) error {
templs, err := LoadProcessedTemplates(ctx, config, openshiftUsername, nsBaseName)
if err != nil {
return err
}
Expand All @@ -26,9 +26,8 @@ func RawInitTenant(ctx context.Context, config Config, callback Callback, opensh
userOpts := ApplyOptions{Config: config.WithToken(usertoken), Callback: callback}
var wg sync.WaitGroup
wg.Add(len(mapped))
username := env.RetrieveUserName(openshiftUsername)
for key, val := range mapped {
namespaceType := tenant.GetNamespaceType(key, username)
namespaceType := tenant.GetNamespaceType(key, nsBaseName)
if namespaceType == tenant.TypeUser {
go func(namespace string, objects env.Objects, opts, userOpts ApplyOptions) {
defer wg.Done()
Expand Down Expand Up @@ -88,8 +87,8 @@ func RawInitTenant(ctx context.Context, config Config, callback Callback, opensh
return nil
}

func RawUpdateTenant(ctx context.Context, config Config, callback Callback, username string) error {
templs, err := LoadProcessedTemplates(ctx, config, username)
func RawUpdateTenant(ctx context.Context, config Config, callback Callback, osUsername, nsBaseName string) error {
templs, err := LoadProcessedTemplates(ctx, config, osUsername, nsBaseName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion openshift/init_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestNumberOfCallsToCluster(t *testing.T) {
objectsInTemplates := tmplObjects(t, data)

// when
err := openshift.RawInitTenant(context.Background(), config, emptyCallback, "developer", "12345")
err := openshift.RawInitTenant(context.Background(), config, emptyCallback, "developer", "developer", "12345")

// then
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions openshift/process_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ func IsNotOfKind(kinds ...string) FilterFunc {
}
}

func LoadProcessedTemplates(ctx context.Context, config Config, username string) (environment.Objects, error) {
func LoadProcessedTemplates(ctx context.Context, config Config, osUsername, nsBaseName string) (environment.Objects, error) {

envService := environment.NewService(config.TemplatesRepo, config.TemplatesRepoBlob, config.TemplatesRepoDir)
vars := environment.CollectVars(username, config.MasterUser, config.OriginalConfig)
vars := environment.CollectVars(osUsername, nsBaseName, config.MasterUser, config.OriginalConfig)
var objs environment.Objects

for _, envType := range environment.DefaultEnvTypes {
Expand Down
2 changes: 1 addition & 1 deletion openshift/process_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestPresenceOfTemplateObjects(t *testing.T) {

func tmplObjects(t *testing.T, data *configuration.Data) environment.Objects {
config := openshift.Config{OriginalConfig: data, MasterUser: "master"}
templs, err := openshift.LoadProcessedTemplates(context.Background(), config, "developer")
templs, err := openshift.LoadProcessedTemplates(context.Background(), config, "developer", "developer")
assert.NoError(t, err)
return templs
}
Expand Down
61 changes: 61 additions & 0 deletions tenant/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package tenant
import (
"fmt"

"github.com/fabric8-services/fabric8-tenant/environment"
"github.com/fabric8-services/fabric8-wit/errors"
"github.com/jinzhu/gorm"
errs "github.com/pkg/errors"
Expand All @@ -20,6 +21,8 @@ type Service interface {
SaveTenant(tenant *Tenant) error
SaveNamespace(namespace *Namespace) error
DeleteAll(tenantID uuid.UUID) error
NamespaceExists(nsName string) (bool, error)
ExistsWithNsBaseName(nsBaseName string) (bool, error)
}

func NewDBService(db *gorm.DB) Service {
Expand All @@ -39,6 +42,18 @@ func (s DBService) Exists(tenantID uuid.UUID) bool {
return true
}

func (s DBService) ExistsWithNsBaseName(nsBaseName string) (bool, error) {
var t Tenant
err := s.db.Table(t.TableName()).Where("ns_base_name = ?", nsBaseName).Find(&t).Error
if err != nil {
if gorm.ErrRecordNotFound == err {
return false, nil
}
return false, err
}
return true, nil
}

func (s DBService) GetTenant(tenantID uuid.UUID) (*Tenant, error) {
var t Tenant
err := s.db.Table(t.TableName()).Where("id = ?", tenantID).Find(&t).Error
Expand All @@ -51,6 +66,18 @@ func (s DBService) GetTenant(tenantID uuid.UUID) (*Tenant, error) {
return &t, nil
}

func (s DBService) NamespaceExists(nsName string) (bool, error) {
var ns Namespace
err := s.db.Table(Namespace{}.TableName()).Where("name = ?", nsName).Find(&ns).Error
if err != nil {
if gorm.ErrRecordNotFound == err {
return false, nil
}
return false, err
}
return true, nil
}

func (s DBService) LookupTenantByClusterAndNamespace(masterURL, namespace string) (*Tenant, error) {
// select t.id from tenant t, namespaces n where t.id = n.tenant_id and n.master_url = ? and n.name = ?
query := fmt.Sprintf("select t.* from %[1]s t, %[2]s n where t.id = n.tenant_id and n.master_url = ? and n.name = ?", Tenant{}.TableName(), Namespace{}.TableName())
Expand Down Expand Up @@ -145,3 +172,37 @@ func (s NilService) LookupTenantByClusterAndNamespace(masterURL, namespace strin
func (s NilService) DeleteAll(tenantID uuid.UUID) error {
return nil
}

func ConstructNsBaseName(repo Service, username string) (string, error) {
return constructNsBaseName(repo, username, 1)
}

func constructNsBaseName(repo Service, username string, number int) (string, error) {
nsBaseName := username
if number > 1 {
nsBaseName += fmt.Sprintf("%d", number)
}
exists, err := repo.ExistsWithNsBaseName(nsBaseName)
if err != nil {
return "", errs.Wrapf(err, "getting already existing tenants with the NsBaseName %s failed: ", nsBaseName)
}
if exists {
number++
return constructNsBaseName(repo, username, number)
}
for _, nsType := range environment.DefaultEnvTypes {
nsName := nsBaseName
if nsType != "user" {
nsName += "-" + nsType
}
exists, err := repo.NamespaceExists(nsName)
if err != nil {
return "", errs.Wrapf(err, "getting already existing namespaces with the name %s failed: ", nsName)
}
if exists {
number++
return constructNsBaseName(repo, username, number)
}
}
return nsBaseName, nil
}
Loading