fix(CRT-355): update non-compliant usernames for existing users when …

…no namespace is provisioned (#779)

controller/tenant.go

@@ -2,6 +2,8 @@ package controller
 
 import (
 	"context"
+	"strings"
+
 	"github.com/fabric8-services/fabric8-common/errors"
 	"github.com/fabric8-services/fabric8-common/log"
 	"github.com/fabric8-services/fabric8-tenant/app"
@@ -17,7 +19,6 @@ import (
 	"github.com/goadesign/goa"
 	errs "github.com/pkg/errors"
 	"github.com/satori/go.uuid"
-	"strings"
 )
 
 // TenantController implements the tenant resource.
@@ -189,11 +190,30 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error {
 	}
 
 	// check if any environment type is missing - should be provisioned
-	missing, _ := filterMissingAndExisting(namespaces)
+	missing, existing := filterMissingAndExisting(namespaces)
 	if len(missing) == 0 {
 		return ctx.Conflict()
 	}
 
+	if len(existing) == 0 && isNotCompliant(dbTenant.NsBaseName) {
+		nsBaseName, err := tenant.ConstructNsBaseName(c.tenantService, environment.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))
+		}
+		dbTenant.NsBaseName = nsBaseName
+		err = tenantRepository.SaveTenant(dbTenant)
+		if err != nil {
+			log.Error(ctx, map[string]interface{}{
+				"err": err,
+			}, "unable to update tenant with the new nsBaseName")
+			return jsonapi.JSONErrorResponse(ctx, err)
+		}
+	}
+
 	clusterNsMapping, err := c.clusterService.GetUserClusterForType(ctx, user)
 	if err != nil {
 		log.Error(ctx, map[string]interface{}{
@@ -224,6 +244,10 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error {
 	return ctx.Accepted()
 }
 
+func isNotCompliant(nsBaseName string) bool {
+	return strings.HasPrefix(nsBaseName, "-") || strings.HasSuffix(nsBaseName, "-") || environment.OnlyNumbers.MatchString(nsBaseName)
+}
+
 // Show runs the show action.
 func (c *TenantController) Show(ctx *app.ShowTenantContext) error {
 	// get user info

controller/tenant_test.go

@@ -3,6 +3,12 @@ package controller_test
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"sync"
+	"testing"
+
 	"github.com/fabric8-services/fabric8-tenant/app"
 	apptest "github.com/fabric8-services/fabric8-tenant/app/test"
 	"github.com/fabric8-services/fabric8-tenant/configuration"
@@ -20,11 +26,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"gopkg.in/h2non/gock.v1"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"sync"
-	"testing"
 )
 
 type TenantControllerTestSuite struct {
@@ -240,6 +241,46 @@ func (s *TenantControllerTestSuite) TestSetupTenantOKWhenAlreadyExists() {
 	assert.Equal(s.T(), totalNumber-(len(cheObjects)+numberOfGetChecksForChe+1), calls)
 }
 
+func (s *TenantControllerTestSuite) TestSetupTenantOKWhenAlreadyExistsButWithUserNameStoredWithDashes() {
+	// given
+	defer gock.OffAll()
+	fxt := tf.FillDB(s.T(), s.DB, tf.AddSpecificTenants(tf.SingleWithName("-johny-")), tf.AddNamespaces())
+	id := fxt.Tenants[0].ID
+	svc, ctrl, config, reset := s.newTestTenantController()
+	defer reset()
+	calls := 0
+	testdoubles.MockPostRequestsToOS(&calls, test.ClusterURL, environment.DefaultEnvTypes, "os-johny-io")
+	ctx := testdoubles.CreateAndMockUserWithUsernameAndTokenPersisted(s.T(), id.String(), "-johny-", false)
+
+	// when
+	apptest.SetupTenantAccepted(s.T(), ctx, svc, ctrl)
+
+	// then
+	totalNumber := testdoubles.ExpectedNumberOfCallsWhenPost(s.T(), config)
+	assert.Equal(s.T(), totalNumber, calls)
+}
+
+func (s *TenantControllerTestSuite) TestSetupTenantOKWhenAlreadyExistsButWithUserNameStoredAsNumber() {
+	// given
+	defer gock.OffAll()
+	fxt := tf.FillDB(s.T(), s.DB, tf.AddSpecificTenants(tf.SingleWithName("12345")), tf.AddNamespaces(environment.TypeChe))
+	id := fxt.Tenants[0].ID
+	svc, ctrl, config, reset := s.newTestTenantController()
+	defer reset()
+	calls := 0
+	testdoubles.MockPostRequestsToOS(&calls, test.ClusterURL, environment.DefaultEnvTypes, "12345")
+	ctx := testdoubles.CreateAndMockUserWithUsernameAndTokenPersisted(s.T(), id.String(), "12345", false)
+
+	// when
+	apptest.SetupTenantAccepted(s.T(), ctx, svc, ctrl)
+
+	// then
+	totalNumber := testdoubles.ExpectedNumberOfCallsWhenPost(s.T(), config)
+	cheObjects := testdoubles.SingleTemplatesObjectsWithDefaults(s.T(), config, environment.TypeChe)
+	numberOfGetChecksForChe := testdoubles.NumberOfGetChecks(cheObjects)
+	assert.Equal(s.T(), totalNumber-(len(cheObjects)+numberOfGetChecksForChe+1), calls)
+}
+
 func (s *TenantControllerTestSuite) TestSetupUnauthorizedFailures() {
 
 	defer gock.OffAll()

environment/template.go

@@ -119,6 +119,7 @@ type Template struct {
 var (
 	specialCharRegexp = regexp.MustCompile("[^a-z0-9]")
 	variableRegexp    = regexp.MustCompile(`\${([A-Z_0-9]+)}`)
+	OnlyNumbers       = regexp.MustCompile("^[0-9]*$")
 )
 
 func newTemplate(filename string, defaultParams map[string]string, version string) Template {
@@ -202,8 +203,8 @@ func RetrieveUserName(openshiftUsername string) string {
 	if strings.HasSuffix(userName, "-") {
 		userName = userName + "io"
 	}
-	matched, err := regexp.MatchString("^[0-9]*$", userName)
-	if matched || err != nil {
+	matched := OnlyNumbers.MatchString(userName)
+	if matched {
 		userName = "os-" + userName + "-io"
 	}
 	return userName

test/doubles/controller.go

@@ -3,6 +3,9 @@ package testdoubles
 import (
 	"context"
 	"fmt"
+	"testing"
+	"time"
+
 	"github.com/fabric8-services/fabric8-tenant/auth"
 	"github.com/fabric8-services/fabric8-tenant/cluster"
 	"github.com/fabric8-services/fabric8-tenant/configuration"
@@ -11,8 +14,6 @@ import (
 	goajwt "github.com/goadesign/goa/middleware/security/jwt"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/h2non/gock.v1"
-	"testing"
-	"time"
 )
 
 func CreateAndMockUserAndToken(t *testing.T, sub string, internal bool) context.Context {
@@ -25,12 +26,21 @@ func CreateAndMockUserAndTokenPersisted(t *testing.T, sub string, internal bool)
 	return CreateAndMockUser(t, sub, internal, true)
 }
 
+func CreateAndMockUserWithUsernameAndTokenPersisted(t *testing.T, sub, username string, internal bool) context.Context {
+	createTokenMockWithUsername(sub, username, true)
+	return CreateAndMockUserWithUsername(t, sub, username, internal, true)
+}
+
 func CreateAndMockUser(t *testing.T, sub string, internal, persist bool) context.Context {
+	return CreateAndMockUserWithUsername(t, sub, "johny", internal, persist)
+}
+
+func CreateAndMockUserWithUsername(t *testing.T, sub, username string, internal, persist bool) context.Context {
 	userToken, err := test.NewToken(
 		map[string]interface{}{
 			"sub":                sub,
-			"preferred_username": "johny",
-			"email":              "johny@redhat.com",
+			"preferred_username": username,
+			"email":              username + "@redhat.com",
 		},
 		"../test/private_key.pem",
 	)
@@ -40,11 +50,11 @@ func CreateAndMockUser(t *testing.T, sub string, internal, persist bool) context
 		featureLevel = auth.InternalFeatureLevel
 	}
 
-	createUserMock(sub, featureLevel, persist)
+	createUserMock(sub, username, featureLevel, persist)
 	return goajwt.WithJWT(context.Background(), userToken)
 }
 
-func createUserMock(tenantId string, featureLevel string, persist bool) {
+func createUserMock(tenantId, username string, featureLevel string, persist bool) {
 	matcher := gock.New("http://authservice").
 		Get("/api/users/" + tenantId).
 		SetMatcher(test.ExpectRequest(test.HasJWTWithSub("tenant_service")))
@@ -57,13 +67,18 @@ func createUserMock(tenantId string, featureLevel string, persist bool) {
            		"attributes": {
                   "identityID": "%s",
            		  "cluster": "%s",
-           		  "email": "johny@redhat.com",
+           		  "email": "%s@redhat.com",
                   "featureLevel": "%s"
            		}
            	  }
-           	}`, tenantId, test.Normalize(test.ClusterURL), featureLevel))
+           	}`, tenantId, test.Normalize(test.ClusterURL), username, featureLevel))
 }
+
 func createTokenMock(tenantId string, persist bool) {
+	createTokenMockWithUsername(tenantId, "johny", persist)
+}
+
+func createTokenMockWithUsername(tenantId, username string, persist bool) {
 	matcher := gock.New("http://authservice").
 		Get("/api/token").
 		MatchParam("for", test.ClusterURL).
@@ -74,11 +89,11 @@ func createTokenMock(tenantId string, persist bool) {
 	}
 	matcher.
 		Reply(200).
-		BodyString(`{ 
+		BodyString(fmt.Sprintf(`{ 
       "token_type": "bearer",
-      "username": "johny@redhat.com",
+      "username": "%s@redhat.com",
       "access_token": "jA0ECQMCWbHrs0GtZQlg0sDQAYMwVoNofrjMocCLv5+FR4GkCPEOiKvK6ifRVsZ6VWLcBVF5k/MFO0Y3EmE8O77xDFRvA9AVPETb7M873tGXMEmqFjgpWvppN81zgmk/enaeJbTBeYhXScyShw7G7kIbgaRy2ufPzVj7f2muM0PHRS334xOVtWZIuaq4lP7EZvW4u0JinSVT0oIHBoCKDFlMlNS1sTygewyI3QOX1quLEEhaDr6/eTG66aTfqMYZQpM4B+m78mi02GLPx3Z24DpjzgshagmGQ8f2kj49QA0LbbFaCUvpqlyStkXNwFm7z+Vuefpp+XYGbD+8MfOKsQxDr7S6ziEdjs+zt/QAr1ZZyoPsC4TaE6kkY1JHIIcrdO5YoX6mbxDMdkLY1ybMN+qMNKtVW4eV9eh34fZKUJ6sjTfdaZ8DjN+rGDKMtZDqwa1h+YYz938jl/bRBEQjK479o7Y6Iu/v4Rwn4YjM4YGjlXs/T/rUO1uye3AWmVNFfi6GtqNpbsKEbkr80WKOOWiSuYeZHbXA7pWMit17U9LtUA=="
-    }`)
+    }`, username))
 }
 
 func PrepareConfigClusterAndAuthService(t *testing.T) (cluster.Service, auth.Service, *configuration.Data, func()) {