Repository: ovh/cds
Pull request #5467 (2020-10-01 09:01:32)

fix(engine): manage signout
Signed-off-by: Yvonnick Esnault <yvonnick.esnault@corp.ovh.com>

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=SECURITY_HOTSPOT) to review)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL)

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=ovh_cds&pullRequest=5467&metric=coverage&view=list) No Coverage information  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=ovh_cds&pullRequest=5467&metric=duplicated_lines_density&view=list) No Duplication information


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=BUG)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=SECURITY_HOTSPOT) to review)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=ovh_cds&pullRequest=5467&resolved=false&types=CODE_SMELL)

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=ovh_cds&pullRequest=5467&metric=coverage&view=list) No Coverage information  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=ovh_cds&pullRequest=5467&metric=duplicated_lines_density&view=list) No Duplication information



Patch:
diff --git a/docs/content/docs/concepts/variables.md b/docs/content/docs/concepts/variables.md
index 18059be49..49ed6709a 100644
--- a/docs/content/docs/concepts/variables.md
+++ b/docs/content/docs/concepts/variables.md
@@ -46,7 +46,7 @@ Here is the list of builtin variables, generated for every build:
 - `{{.cds.job}}` The name of the current job
 - `{{.cds.manual}}` true if current pipeline is manually run, false otherwise
 - `{{.cds.pipeline}}` The name of the current pipeline
-- `{{.cds.project}}` The name of the current project
+- `{{.cds.project}}` The key of the current project
 - `{{.cds.run}}` Run Number of current workflow, example: 3.0
 - `{{.cds.run.number}}` Number of current workflow, example: 3 if `{{.cds.run}} = 3.0`
 - `{{.cds.run.subnumber}}` Sub Number of current workflow, example: 4 if `{{.cds.run}} = 3.4`
diff --git a/engine/api/mail/mail.go b/engine/api/mail/mail.go
index 69650c59d..26a774e24 100644
--- a/engine/api/mail/mail.go
+++ b/engine/api/mail/mail.go
@@ -90,28 +90,30 @@ func smtpClient(ctx context.Context) (*smtp.Client, error) {
 		// Here is the key, you need to call tls.Dial instead of smtp.Dial
 		// for smtp servers running on 465 that require an ssl connection
 		// from the very beginning (no starttls)
-		conn, errc := tls.Dial("tcp", servername, tlsconfig)
-		if errc != nil {
-			log.Warning(ctx, "Error with c.Dial:%s\n", errc.Error())
-			return nil, errc
+		conn, err := tls.Dial("tcp", servername, tlsconfig)
+		if err != nil {
+			log.Warning(ctx, "Error with c.Dial:%s\n", err.Error())
+			return nil, sdk.WithStack(err)
 		}
 
 		c, err = smtp.NewClient(conn, smtpHost)
 		if err != nil {
 			log.Warning(ctx, "Error with c.NewClient:%s\n", err.Error())
-			return nil, err
+			return nil, sdk.WithStack(err)
 		}
 		// TLS config
 		tlsconfig := &tls.Config{
 			InsecureSkipVerify: false,
 			ServerName:         smtpHost,
 		}
-		c.StartTLS(tlsconfig)
+		if err := c.StartTLS(tlsconfig); err != nil {
+			return nil, sdk.WithStack(err)
+		}
 	} else {
 		c, err = smtp.Dial(servername)
 		if err != nil {
 			log.Warning(ctx, "Error with c.NewClient:%s\n", err.Error())
-			return nil, err
+			return nil, sdk.WithStack(err)
 		}
 	}
 
@@ -183,7 +185,12 @@ func SendEmail(ctx context.Context, subject string, mailContent *bytes.Buffer, u
 	headers := make(map[string]string)
 	headers["From"] = smtpFrom
 	headers["To"] = to.String()
-	headers["Subject"] = subject
+	if sdk.StringIsAscii(subject) {
+		headers["Subject"] = subject
+	} else {
+		// https://tools.ietf.org/html/rfc2047
+		headers["Subject"] = "=?UTF-8?Q?" + subject + "?="
+	}
 
 	// https://tools.ietf.org/html/rfc4021
 	headers["Date"] = time.Now().Format(time.RFC1123Z)
diff --git a/engine/api/repositories_manager.go b/engine/api/repositories_manager.go
index f54242e9a..614f6597b 100644
--- a/engine/api/repositories_manager.go
+++ b/engine/api/repositories_manager.go
@@ -14,7 +14,6 @@ import (
 	"github.com/ovh/cds/engine/api/event"
 	"github.com/ovh/cds/engine/api/project"
 	"github.com/ovh/cds/engine/api/repositoriesmanager"
-	"github.com/ovh/cds/engine/api/services"
 	"github.com/ovh/cds/engine/api/workflow"
 	"github.com/ovh/cds/engine/cache"
 	"github.com/ovh/cds/engine/service"
@@ -135,19 +134,11 @@ func (api *API) repositoriesManagerOAuthCallbackHandler() service.Handler {
 		consumerID := data["consumer_id"]
 
 		authConsumer, err := authentication.LoadConsumerByID(ctx, api.mustDB(), consumerID,
-			authentication.LoadConsumerOptions.WithAuthentifiedUser,
-			authentication.LoadConsumerOptions.WithConsumerGroups)
+			authentication.LoadConsumerOptions.WithAuthentifiedUser)
 		if err != nil {
 			return sdk.WrapError(sdk.ErrForbidden, "repositoriesManagerAuthorizeCallback> Error")
 		}
 
-		// Add service for consumer if exists
-		s, err := services.LoadByConsumerID(context.Background(), api.mustDB(), authConsumer.ID)
-		if err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {
-			return sdk.WrapError(sdk.ErrForbidden, "repositoriesManagerAuthorizeCallback> Error")
-		}
-		authConsumer.Service = s
-
 		proj, errP := project.Load(ctx, api.mustDB(), projectKey)
 		if errP != nil {
 			return sdk.WrapError(errP, "repositoriesManagerAuthorizeCallback> Cannot load project")
diff --git a/engine/api/worker/init.go b/engine/api/worker/init.go
index a2008a59e..c74dc9451 100644
--- a/engine/api/worker/init.go
+++ b/engine/api/worker/init.go
@@ -19,7 +19,7 @@ func Initialize(c context.Context, DBFunc func() *gorp.DbMap, store cache.Store)
 		select {
 		case <-c.Done():
 			if c.Err() != nil {
-				log.Error(c, "Exiting workflow ticker: %v", c.Err())
+				log.Error(c, "Exiting worker ticker: %v", c.Err())
 				return nil
 			}
 		case <-tickHeart.C:
@@ -31,7 +31,7 @@ func Initialize(c context.Context, DBFunc func() *gorp.DbMap, store cache.Store)
 
 			go func() {
 				if err := DisableDeadWorkers(c, db); err != nil {
-					log.Warning(c, "workflow.disableDeadWorkers> Error on disableDeadWorkers : %v", err)
+					log.Warning(c, "worker.disableDeadWorkers> Error on disableDeadWorkers : %v", err)
 				}
 			}()
 		}
diff --git a/engine/cmd_start.go b/engine/cmd_start.go
index a2e7cfae3..e02d9fad7 100644
--- a/engine/cmd_start.go
+++ b/engine/cmd_start.go
@@ -50,6 +50,12 @@ var (
 	flagStartVaultToken      string
 )
 
+type serviceConf struct {
+	arg     string
+	service service.Service
+	cfg     interface{}
+}
+
 var startCmd = &cobra.Command{
 	Use:   "start",
 	Short: "Start CDS",
@@ -113,21 +119,6 @@ See $ engine config command for more details.
 		// initialize context
 		defer cancel()
 
-		// gracefully shutdown all
-		c := make(chan os.Signal, 1)
-		signal.Notify(c, os.Interrupt, syscall.SIGTERM)
-		go func() {
-			<-c
-			signal.Stop(c)
-			cancel()
-		}()
-
-		type serviceConf struct {
-			arg     string
-			service service.Service
-			cfg     interface{}
-		}
-
 		var (
 			serviceConfs []serviceConf
 			names        []string
@@ -255,6 +246,16 @@ See $ engine config command for more details.
 			}
 		}
 
+		// gracefully shutdown all
+		c := make(chan os.Signal, 1)
+		signal.Notify(c, os.Interrupt, syscall.SIGTERM)
+		go func(ctx context.Context) {
+			<-c
+			unregisterServices(ctx, serviceConfs)
+			signal.Stop(c)
+			cancel()
+		}(ctx)
+
 		//Initialize logs
 		logConf := log.Conf{
 			Level:                      conf.Log.Level,
@@ -313,12 +314,25 @@ See $ engine config command for more details.
 
 		//Wait for the end
 		<-ctx.Done()
-		if ctx.Err() != nil {
-			fmt.Printf("Exiting (%v)\n", ctx.Err())
-		}
+
 	},
 }
 
+func unregisterServices(ctx context.Context, serviceConfs []serviceConf) {
+	// unregister all services
+	for i := range serviceConfs {
+		s := serviceConfs[i]
+		fmt.Printf("Unregister (%v)\n", s.service.Name())
+		if err := s.service.Unregister(ctx); err != nil {
+			log.Error(ctx, "%s> Unable to unregister: %v", s.service.Name(), err)
+		}
+	}
+
+	if ctx.Err() != nil {
+		fmt.Printf("Exiting (%v)\n", ctx.Err())
+	}
+}
+
 func start(c context.Context, s service.Service, cfg interface{}, serviceName string) {
 	if err := serve(c, s, serviceName, cfg); err != nil {
 		fmt.Printf("Service has been stopped: %s %+v", serviceName, err)
diff --git a/engine/service/service.go b/engine/service/service.go
index 0386c40db..cc124cb6d 100644
--- a/engine/service/service.go
+++ b/engine/service/service.go
@@ -104,6 +104,7 @@ loop:
 	return nil
 }
 
+// Register registers a new service on API
 func (c *Common) Register(ctx context.Context, cfg sdk.ServiceConfig) error {
 	log.Info(ctx, "Registing service %s(%T) %s", c.Type(), c, c.Name())
 
@@ -140,6 +141,17 @@ func (c *Common) Register(ctx context.Context, cfg sdk.ServiceConfig) error {
 	return nil
 }
 
+// Unregister logout the service
+func (c *Common) Unregister(ctx context.Context) error {
+	// no logout needed for api
+	if c.ServiceType == "api" {
+		return nil
+	}
+
+	log.Info(ctx, "Unregisting service %s(%T) %s", c.Type(), c, c.Name())
+	return c.Client.AuthConsumerSignout()
+}
+
 // Heartbeat have to be launch as a goroutine, call DoHeartBeat each 30s
 func (c *Common) Heartbeat(ctx context.Context, status func(ctx context.Context) *sdk.MonitoringStatus) error {
 	// no heartbeat for api
diff --git a/engine/service/types.go b/engine/service/types.go
index e675dbb96..b50b64d74 100644
--- a/engine/service/types.go
+++ b/engine/service/types.go
@@ -120,6 +120,7 @@ type Service interface {
 	Start(ctx context.Context, cfg cdsclient.ServiceConfig) error
 	Init(cfg interface{}) (cdsclient.ServiceConfig, error)
 	Register(ctx context.Context, cfg sdk.ServiceConfig) error
+	Unregister(ctx context.Context) error
 	Heartbeat(ctx context.Context, status func(ctx context.Context) *sdk.MonitoringStatus) error
 	Status(ctx context.Context) *sdk.MonitoringStatus
 	NamedService
diff --git a/sdk/cdsclient/http_websocket.go b/sdk/cdsclient/http_websocket.go
index c6f6f0fcc..9430f6a8b 100644
--- a/sdk/cdsclient/http_websocket.go
+++ b/sdk/cdsclient/http_websocket.go
@@ -60,7 +60,7 @@ func (c *client) RequestWebsocket(ctx context.Context, goRoutines *sdk.GoRoutine
 		for {
 			select {
 			case <-ctx.Done():
-				log.Warning(wsContext, "Leaving....")
+				log.Warning(wsContext, "websocket goroutine leaving....")
 				return
 			case m := <-msgToSend:
 				if err := con.WriteJSON(m); err != nil {
diff --git a/sdk/common.go b/sdk/common.go
index 266d636d2..59b914ad3 100644
--- a/sdk/common.go
+++ b/sdk/common.go
@@ -267,6 +267,15 @@ func (s *Int64Slice) Remove(v int64) {
 	*s = filtered
 }
 
+func StringIsAscii(s string) bool {
+	for _, r := range s {
+		if r > unicode.MaxASCII {
+			return false
+		}
+	}
+	return true
+}
+
 func RemoveNotPrintableChar(in string) string {
 	m := func(r rune) rune {
 		switch {


Test patch:
diff --git a/sdk/common_test.go b/sdk/common_test.go
index acd62275e..55d672c74 100644
--- a/sdk/common_test.go
+++ b/sdk/common_test.go
@@ -7,6 +7,11 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestStringIsAscii(t *testing.T) {
+	assert.True(t, StringIsAscii("aaa"))
+	assert.False(t, StringIsAscii("aaa 🚀"))
+}
+
 func TestRemoveNotPrintableChar(t *testing.T) {
 	var tests = []struct {
 		in  string