From eda438aaed5e0d5ba537a9313c8e37d7cd02ad54 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 29 Jun 2020 17:57:06 +0200 Subject: [PATCH 1/4] Added basic support for service principal login, run ACI e2e tests with it --- .github/workflows/ci.yml | 7 ++++++ azure/login/login.go | 41 +++++++++++++++++++++++++++++++++++ cli/mobycli/exec_test.go | 3 ++- go.mod | 1 + go.sum | 2 ++ tests/aci-e2e/e2e-aci_test.go | 14 ++++++++++++ 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47e16aa3..3b2f3514 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,3 +55,10 @@ jobs: - name: E2E Test run: make e2e-local + + - name: ACI e2e Test + env: + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + run: make e2e-aci \ No newline at end of file diff --git a/azure/login/login.go b/azure/login/login.go index 50ccdc48..c072f6eb 100644 --- a/azure/login/login.go +++ b/azure/login/login.go @@ -31,6 +31,7 @@ import ( "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/adal" + auth2 "github.com/Azure/go-autorest/autorest/azure/auth" "github.com/Azure/go-autorest/autorest/azure/cli" "github.com/Azure/go-autorest/autorest/date" "github.com/pkg/errors" @@ -93,6 +94,31 @@ func newAzureLoginServiceFromPath(tokenStorePath string, helper apiHelper) (Azur }, nil } +// LoginFromServicePrincipal login with clientId / clientSecret from a previously created service principal +func (login AzureLoginService) LoginFromServicePrincipal(clientID string, clientSecret string, tenantID string) error { + // Tried with auth2.NewUsernamePasswordConfig() but could not make this work with username / password, setting this for CI with clientID / clientSecret + creds := auth2.NewClientCredentialsConfig(clientID, clientSecret, tenantID) + + spToken, err := creds.ServicePrincipalToken() + if err != nil { + return errors.Wrapf(errdefs.ErrLoginFailed, "could not login with service principal: %s", err) + } + err = spToken.Refresh() + if err != nil { + return errors.Wrapf(errdefs.ErrLoginFailed, "could not login with service principal: %s", err) + } + token, err := spToOAuthToken(spToken.Token()) + if err != nil { + return errors.Wrapf(errdefs.ErrLoginFailed, "could not read service principal token expiry: %s", err) + } + loginInfo := TokenInfo{TenantID: tenantID, Token: token} + + if err := login.tokenStore.writeLoginInfo(loginInfo); err != nil { + return errors.Wrapf(errdefs.ErrLoginFailed, "could not store login info: %s", err) + } + return nil +} + // Login performs an Azure login through a web browser func (login AzureLoginService) Login(ctx context.Context) error { queryCh := make(chan localResponse, 1) @@ -179,6 +205,21 @@ func toOAuthToken(token azureToken) oauth2.Token { return oauthToken } +func spToOAuthToken(token adal.Token) (oauth2.Token, error) { + expiresIn, err := token.ExpiresIn.Int64() + if err != nil { + return oauth2.Token{}, err + } + expireTime := time.Now().Add(time.Duration(expiresIn) * time.Second) + oauthToken := oauth2.Token{ + RefreshToken: token.RefreshToken, + AccessToken: token.AccessToken, + Expiry: expireTime, + TokenType: token.Type, + } + return oauthToken, nil +} + // NewAuthorizerFromLogin creates an authorizer based on login access token func NewAuthorizerFromLogin() (autorest.Authorizer, error) { return newAuthorizerFromLoginStorePath(getTokenStorePath()) diff --git a/cli/mobycli/exec_test.go b/cli/mobycli/exec_test.go index b21a2fa5..e9d0a1b6 100644 --- a/cli/mobycli/exec_test.go +++ b/cli/mobycli/exec_test.go @@ -3,9 +3,10 @@ package mobycli import ( "testing" - "github.com/docker/api/tests/framework" . "github.com/onsi/gomega" "github.com/stretchr/testify/suite" + + "github.com/docker/api/tests/framework" ) type MobyExecSuite struct { diff --git a/go.mod b/go.mod index 50538883..5c29b7e8 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Azure/azure-storage-file-go v0.7.0 github.com/Azure/go-autorest/autorest v0.11.0 github.com/Azure/go-autorest/autorest/adal v0.9.0 + github.com/Azure/go-autorest/autorest/azure/auth v0.5.0 github.com/Azure/go-autorest/autorest/azure/cli v0.4.0 github.com/Azure/go-autorest/autorest/date v0.3.0 github.com/Azure/go-autorest/autorest/to v0.4.0 diff --git a/go.sum b/go.sum index 32ff8c66..e255df7c 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,8 @@ github.com/Azure/go-autorest/autorest v0.11.0/go.mod h1:JFgpikqFJ/MleTTxwepExTKn github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0= github.com/Azure/go-autorest/autorest/adal v0.9.0 h1:SigMbuFNuKgc1xcGhaeapbh+8fgsu+GxgDRFyg7f5lM= github.com/Azure/go-autorest/autorest/adal v0.9.0/go.mod h1:/c022QCutn2P7uY+/oQWWNcK9YU+MH96NgK+jErpbcg= +github.com/Azure/go-autorest/autorest/azure/auth v0.5.0 h1:nSMjYIe24eBYasAIxt859TxyXef/IqoH+8/g4+LmcVs= +github.com/Azure/go-autorest/autorest/azure/auth v0.5.0/go.mod h1:QRTvSZQpxqm8mSErhnbI+tANIBAKP7B+UIE2z4ypUO0= github.com/Azure/go-autorest/autorest/azure/cli v0.4.0 h1:Ml+UCrnlKD+cJmSzrZ/RDcDw86NjkRUpnFh7V5JUhzU= github.com/Azure/go-autorest/autorest/azure/cli v0.4.0/go.mod h1:JljT387FplPzBA31vUcvsetLKF3pec5bdAxjVU4kI2s= github.com/Azure/go-autorest/autorest/date v0.1.0/go.mod h1:plvfp3oPSKwf2DNjlBjWF/7vwR+cUD/ELuzDCXwHUVA= diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 744f7ca7..2d7292de 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -20,10 +20,12 @@ import ( "context" "fmt" "net/url" + "os" "strings" "testing" "github.com/docker/api/azure" + "github.com/docker/api/azure/login" "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" azure_storage "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage" @@ -69,6 +71,18 @@ func (s *E2eACISuite) TestContextDefault() { } func (s *E2eACISuite) TestACIBackend() { + + It("Logs in azure using service principal credentials", func() { + login, err := login.NewAzureLoginService() + Expect(err).To(BeNil()) + // in order to create new service principal and get these 3 values : `az ad sp create-for-rbac --name 'TestServicePrincipal' --sdk-auth` + clientID := os.Getenv("AZURE_CLIENT_ID") + clientSecret := os.Getenv("AZURE_CLIENT_SECRET") + tenantID := os.Getenv("AZURE_TENANT_ID") + err = login.LoginFromServicePrincipal(clientID, clientSecret, tenantID) + Expect(err).To(BeNil()) + }) + It("creates a new aci context for tests", func() { setupTestResourceGroup(resourceGroupName) helper := azure.NewACIResourceGroupHelper() From 82ff8dcd7d1f8921d69cf6a3d6ee602dab2b91d1 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 30 Jun 2020 07:44:45 +0200 Subject: [PATCH 2/4] Fix resource limit defaults and make aci e2e tests pass --- azure/convert/convert.go | 10 ++++-- azure/convert/convert_test.go | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/azure/convert/convert.go b/azure/convert/convert.go index a2dae725..447e59b0 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "io/ioutil" + "math" "strconv" "strings" @@ -267,7 +268,9 @@ func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (c memLimit := 1. // Default 1 Gb var cpuLimit float64 = 1 if s.Deploy != nil && s.Deploy.Resources.Limits != nil { - memLimit = float64(bytesToGb(s.Deploy.Resources.Limits.MemoryBytes)) + if s.Deploy.Resources.Limits.MemoryBytes != 0 { + memLimit = bytesToGb(s.Deploy.Resources.Limits.MemoryBytes) + } if s.Deploy.Resources.Limits.NanoCPUs != "" { cpuLimit, err = strconv.ParseFloat(s.Deploy.Resources.Limits.NanoCPUs, 0) if err != nil { @@ -295,8 +298,9 @@ func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (c } -func bytesToGb(b types.UnitBytes) int64 { - return int64(b) / 1024 / 1024 / 1024 // from bytes to gigabytes +func bytesToGb(b types.UnitBytes) float64 { + f := float64(b) / 1024 / 1024 / 1024 // from bytes to gigabytes + return math.Round(f*100) / 100 } // ContainerGroupToContainer composes a Container from an ACI container definition diff --git a/azure/convert/convert_test.go b/azure/convert/convert_test.go index d9f067ec..4953ce2f 100644 --- a/azure/convert/convert_test.go +++ b/azure/convert/convert_test.go @@ -213,6 +213,67 @@ func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerMultiplePorts Expect(*groupPorts[1].Port).To(Equal(int32(8080))) } +func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerResourceLimits() { + _0_1Gb := 0.1 * 1024 * 1024 * 1024 + project := compose.Project{ + Name: "", + Config: types.Config{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + Deploy: &types.DeployConfig{ + Resources: types.Resources{ + Limits: &types.Resource{ + NanoCPUs: "0.1", + MemoryBytes: types.UnitBytes(_0_1Gb), + }, + }, + }, + }, + }, + }, + } + + group, err := ToContainerGroup(suite.ctx, project) + Expect(err).To(BeNil()) + + container1 := (*group.Containers)[0] + limits := *container1.Resources.Limits + Expect(*limits.CPU).To(Equal(float64(0.1))) + Expect(*limits.MemoryInGB).To(Equal(float64(0.1))) +} + +func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerResourceLimitsDefaults() { + project := compose.Project{ + Name: "", + Config: types.Config{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + Deploy: &types.DeployConfig{ + Resources: types.Resources{ + Limits: &types.Resource{ + NanoCPUs: "", + MemoryBytes: 0, + }, + }, + }, + }, + }, + }, + } + + group, err := ToContainerGroup(suite.ctx, project) + Expect(err).To(BeNil()) + + container1 := (*group.Containers)[0] + limits := *container1.Resources.Limits + Expect(*limits.CPU).To(Equal(float64(1))) + Expect(*limits.MemoryInGB).To(Equal(float64(1))) +} + func TestConvertTestSuite(t *testing.T) { RegisterTestingT(t) suite.Run(t, new(ConvertTestSuite)) From 2268f0c598617de878de9fe2bb5c7cb44bcee58a Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 30 Jun 2020 10:11:06 +0200 Subject: [PATCH 3/4] Renamed Login method specific to tests, to make things more explicit --- azure/login/login.go | 5 +++-- tests/aci-e2e/e2e-aci_test.go | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/azure/login/login.go b/azure/login/login.go index c072f6eb..2df64823 100644 --- a/azure/login/login.go +++ b/azure/login/login.go @@ -94,8 +94,9 @@ func newAzureLoginServiceFromPath(tokenStorePath string, helper apiHelper) (Azur }, nil } -// LoginFromServicePrincipal login with clientId / clientSecret from a previously created service principal -func (login AzureLoginService) LoginFromServicePrincipal(clientID string, clientSecret string, tenantID string) error { +// TestLoginFromServicePrincipal login with clientId / clientSecret from a previously created service principal. +// The resulting token does not include a refresh token, used for tests only +func (login AzureLoginService) TestLoginFromServicePrincipal(clientID string, clientSecret string, tenantID string) error { // Tried with auth2.NewUsernamePasswordConfig() but could not make this work with username / password, setting this for CI with clientID / clientSecret creds := auth2.NewClientCredentialsConfig(clientID, clientSecret, tenantID) diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 2d7292de..54471321 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -71,7 +71,6 @@ func (s *E2eACISuite) TestContextDefault() { } func (s *E2eACISuite) TestACIBackend() { - It("Logs in azure using service principal credentials", func() { login, err := login.NewAzureLoginService() Expect(err).To(BeNil()) @@ -79,7 +78,7 @@ func (s *E2eACISuite) TestACIBackend() { clientID := os.Getenv("AZURE_CLIENT_ID") clientSecret := os.Getenv("AZURE_CLIENT_SECRET") tenantID := os.Getenv("AZURE_TENANT_ID") - err = login.LoginFromServicePrincipal(clientID, clientSecret, tenantID) + err = login.TestLoginFromServicePrincipal(clientID, clientSecret, tenantID) Expect(err).To(BeNil()) }) From f3cbbc48b8b133c7af1afd83ac77ae365a805f89 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 30 Jun 2020 10:14:19 +0200 Subject: [PATCH 4/4] Aci e2e tests moved to master build only --- .github/workflows/ci.yml | 9 +-------- .github/workflows/master-ci.yml | 7 +++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b2f3514..99065e8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,11 +54,4 @@ jobs: run: make -f builder.Makefile cli - name: E2E Test - run: make e2e-local - - - name: ACI e2e Test - env: - AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} - AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} - AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} - run: make e2e-aci \ No newline at end of file + run: make e2e-local \ No newline at end of file diff --git a/.github/workflows/master-ci.yml b/.github/workflows/master-ci.yml index 18c7cc0a..d45055d0 100644 --- a/.github/workflows/master-ci.yml +++ b/.github/workflows/master-ci.yml @@ -59,6 +59,13 @@ jobs: - name: E2E Test run: make e2e-local + - name: ACI e2e Test + env: + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + run: make e2e-aci + windows-build: name: Windows Build runs-on: windows-latest