From 36865bc4ef101871ed7226e925f4657a91417273 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Tue, 11 Aug 2020 17:58:36 +0200 Subject: [PATCH] fix(enterprise): fix login flow Fixes a recent regression in the login flow, and stops throwing enterprise-related errors for non-enterprise projects when the user is logged in. Also added unit tests for the `login` command. --- garden-service/src/cli/cli.ts | 6 +- garden-service/src/commands/login.ts | 2 +- garden-service/src/commands/run/workflow.ts | 6 +- .../src/enterprise/buffered-event-stream.ts | 2 +- garden-service/src/enterprise/init.ts | 43 ++++++++-- .../src/enterprise/workflow-lifecycle.ts | 2 +- garden-service/src/garden.ts | 32 +++---- .../test/unit/src/commands/get/get-outputs.ts | 4 + .../test/unit/src/commands/login.ts | 84 +++++++++++++++++++ 9 files changed, 150 insertions(+), 31 deletions(-) create mode 100644 garden-service/test/unit/src/commands/login.ts diff --git a/garden-service/src/cli/cli.ts b/garden-service/src/cli/cli.ts index 8889192cf1..9919d27f07 100644 --- a/garden-service/src/cli/cli.ts +++ b/garden-service/src/cli/cli.ts @@ -43,6 +43,7 @@ import { defaultDotIgnoreFiles } from "../util/fs" import { renderError } from "../logger/renderers" import { getDefaultProfiler } from "../util/profiling" import { BufferedEventStream } from "../enterprise/buffered-event-stream" +import { makeEnterpriseContext } from "../enterprise/init" export async function makeDummyGarden(root: string, gardenOpts: GardenOpts = {}) { const environments = gardenOpts.environmentName @@ -229,11 +230,12 @@ ${renderCommands(commands)} garden = await Garden.factory(root, contextOpts) } - if (garden.enterpriseContext) { + const enterpriseContext = makeEnterpriseContext(garden) + if (enterpriseContext) { log.silly(`Connecting Garden instance to BufferedEventStream`) bufferedEventStream.connect({ + enterpriseContext, eventBus: garden.events, - enterpriseContext: garden.enterpriseContext, environmentName: garden.environmentName, namespace: garden.namespace, }) diff --git a/garden-service/src/commands/login.ts b/garden-service/src/commands/login.ts index 3ba394bc14..46df309273 100644 --- a/garden-service/src/commands/login.ts +++ b/garden-service/src/commands/login.ts @@ -23,7 +23,7 @@ export class LoginCommand extends Command { async action({ garden, log, headerLog }: CommandParams): Promise { printHeader(headerLog, "Login", "cloud") - const enterpriseDomain = garden.enterpriseContext?.enterpriseDomain + const enterpriseDomain = garden.enterpriseDomain if (!enterpriseDomain) { throw new ConfigurationError(`Error: Your project configuration does not specify a domain.`, {}) } diff --git a/garden-service/src/commands/run/workflow.ts b/garden-service/src/commands/run/workflow.ts index be44b5e997..03a565a868 100644 --- a/garden-service/src/commands/run/workflow.ts +++ b/garden-service/src/commands/run/workflow.ts @@ -30,6 +30,7 @@ import { registerWorkflowRun } from "../../enterprise/workflow-lifecycle" import { parseCliArgs, pickCommand, processCliArgs } from "../../cli/helpers" import { StringParameter } from "../../cli/params" import { getAllCommands } from "../commands" +import { makeEnterpriseContext } from "../../enterprise/init" const runWorkflowArgs = { workflow: new StringParameter({ @@ -347,10 +348,11 @@ export function logErrors( } async function registerAndSetUid(garden: Garden, log: LogEntry, config: WorkflowConfig) { - if (garden.enterpriseContext && !gardenEnv.GARDEN_PLATFORM_SCHEDULED) { + const enterpriseContext = makeEnterpriseContext(garden) + if (enterpriseContext && !gardenEnv.GARDEN_PLATFORM_SCHEDULED) { const workflowRunUid = await registerWorkflowRun({ + enterpriseContext, workflowConfig: config, - enterpriseContext: garden.enterpriseContext, environment: garden.environmentName, namespace: garden.namespace, log, diff --git a/garden-service/src/enterprise/buffered-event-stream.ts b/garden-service/src/enterprise/buffered-event-stream.ts index fc9838b906..037a056946 100644 --- a/garden-service/src/enterprise/buffered-event-stream.ts +++ b/garden-service/src/enterprise/buffered-event-stream.ts @@ -14,7 +14,7 @@ import { got } from "../util/http" import { makeAuthHeader } from "./auth" import { LogLevel } from "../logger/log-node" import { gardenEnv } from "../constants" -import { GardenEnterpriseContext } from "../garden" +import { GardenEnterpriseContext } from "./init" export type StreamEvent = { name: EventName diff --git a/garden-service/src/enterprise/init.ts b/garden-service/src/enterprise/init.ts index 4fb7682d6c..1af389d60a 100644 --- a/garden-service/src/enterprise/init.ts +++ b/garden-service/src/enterprise/init.ts @@ -12,9 +12,9 @@ import { LogEntry } from "../logger/log-entry" import { ProjectConfig } from "../config/project" import { readAuthToken, checkClientAuthToken } from "./auth" import { deline } from "../util/string" -import { ConfigurationError } from "../exceptions" import { getSecrets } from "./secrets" import { StringMap } from "../config/common" +import { Garden } from "../garden" export interface EnterpriseInitParams { log: LogEntry @@ -22,6 +22,12 @@ export interface EnterpriseInitParams { environmentName: string } +export interface GardenEnterpriseContext { + clientAuthToken: string + projectId: string + enterpriseDomain: string +} + export async function enterpriseInit({ log, projectConfig, environmentName }: EnterpriseInitParams) { const { id: projectId, domain } = projectConfig const clientAuthToken = await readAuthToken(log) @@ -44,12 +50,11 @@ export async function enterpriseInit({ log, projectConfig, environmentName }: En `) } if (errorMessages.length > 0) { - throw new ConfigurationError( - dedent` + log.verbose( + chalk.gray(dedent` ${errorMessages.join("\n\n")} - Logging out via the ${chalk.bold("garden logout")} command will suppress this message.`, - {} + Logging out via the ${chalk.bold("garden logout")} command will suppress this message.`) ) } } else { @@ -75,3 +80,31 @@ export async function enterpriseInit({ log, projectConfig, environmentName }: En return { clientAuthToken, secrets } } + +/** + * Returns null if one or more parameters are null. + * + * Returns a `GardenEnterpriseContext` otherwise. + */ +export function makeEnterpriseContext(garden: Garden): GardenEnterpriseContext | null { + const missing: string[] = [] + if (!garden.clientAuthToken) { + missing.push("client auth token") + } + if (!garden.projectId) { + missing.push("project id") + } + if (!garden.enterpriseDomain) { + missing.push("domain") + } + if (missing.length > 0) { + garden.log.silly(`Enterprise features disabled. Missing values: ${missing.join(",")}`) + return null + } else { + return { + clientAuthToken: garden.clientAuthToken!, + projectId: garden.projectId!, + enterpriseDomain: garden.enterpriseDomain!, + } + } +} diff --git a/garden-service/src/enterprise/workflow-lifecycle.ts b/garden-service/src/enterprise/workflow-lifecycle.ts index 9f1c2bc81c..14e1a1cce9 100644 --- a/garden-service/src/enterprise/workflow-lifecycle.ts +++ b/garden-service/src/enterprise/workflow-lifecycle.ts @@ -11,7 +11,7 @@ import { makeAuthHeader } from "./auth" import { WorkflowConfig } from "../config/workflow" import { LogEntry } from "../logger/log-entry" import { PlatformError } from "../exceptions" -import { GardenEnterpriseContext } from "../garden" +import { GardenEnterpriseContext } from "./init" export interface RegisterWorkflowRunParams { workflowConfig: WorkflowConfig diff --git a/garden-service/src/garden.ts b/garden-service/src/garden.ts index a87e124872..5636a51c7a 100644 --- a/garden-service/src/garden.ts +++ b/garden-service/src/garden.ts @@ -116,17 +116,12 @@ export interface GardenOpts { variables?: PrimitiveMap } -export interface GardenEnterpriseContext { - clientAuthToken: string - projectId: string - enterpriseDomain: string -} - export interface GardenParams { artifactsPath: string buildDir: BuildDir + clientAuthToken: string | null + enterpriseDomain: string | null projectId: string | null - enterpriseContext: GardenEnterpriseContext | null dotIgnoreFiles: string[] environmentName: string environmentConfigs: EnvironmentConfig[] @@ -165,7 +160,8 @@ export class Garden { private readonly taskGraph: TaskGraph private watcher: Watcher private asyncLock: any - public readonly enterpriseContext: GardenEnterpriseContext | null + public readonly clientAuthToken: string | null + public readonly enterpriseDomain: string | null public readonly projectId: string | null public sessionId: string | null public readonly configStore: ConfigStore @@ -202,7 +198,8 @@ export class Garden { constructor(params: GardenParams) { this.buildDir = params.buildDir - this.enterpriseContext = params.enterpriseContext + this.clientAuthToken = params.clientAuthToken + this.enterpriseDomain = params.enterpriseDomain this.projectId = params.projectId this.sessionId = params.sessionId this.environmentName = params.environmentName @@ -329,25 +326,22 @@ export class Garden { await ensureConnected() const sessionId = opts.sessionId || null - - const { id: projectId, domain: enterpriseDomain } = config - + const projectId = config.id || null let secrets: StringMap = {} - let enterpriseContext: GardenEnterpriseContext | null = null + let clientAuthToken: string | null = null + const enterpriseDomain = config.domain || null if (!opts.noEnterprise) { const enterpriseInitResult = await enterpriseInit({ log, projectConfig: config, environmentName }) secrets = enterpriseInitResult.secrets - const clientAuthToken = enterpriseInitResult.clientAuthToken - if (clientAuthToken && projectId && enterpriseDomain) { - enterpriseContext = { clientAuthToken, projectId, enterpriseDomain } - } + clientAuthToken = enterpriseInitResult.clientAuthToken } const garden = new this({ artifactsPath, sessionId, - projectId: projectId || null, - enterpriseContext, + clientAuthToken, + enterpriseDomain, + projectId, projectRoot, projectName, environmentName, diff --git a/garden-service/test/unit/src/commands/get/get-outputs.ts b/garden-service/test/unit/src/commands/get/get-outputs.ts index dff57c73dd..2ca6ba3e60 100644 --- a/garden-service/test/unit/src/commands/get/get-outputs.ts +++ b/garden-service/test/unit/src/commands/get/get-outputs.ts @@ -37,6 +37,10 @@ describe("GetOutputsCommand", () => { } }) + after(async () => { + await tmpDir.cleanup() + }) + it("should resolve and return defined project outputs", async () => { const plugin = createGardenPlugin({ name: "test", diff --git a/garden-service/test/unit/src/commands/login.ts b/garden-service/test/unit/src/commands/login.ts new file mode 100644 index 0000000000..7e33a2f5df --- /dev/null +++ b/garden-service/test/unit/src/commands/login.ts @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2018-2020 Garden Technologies, Inc. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +import { expect } from "chai" +import td from "testdouble" +import tmp from "tmp-promise" +import { ProjectConfig, defaultNamespace } from "../../../../src/config/project" +import { exec } from "../../../../src/util/util" +import { DEFAULT_API_VERSION } from "../../../../src/constants" +import { TestGarden, withDefaultGlobalOpts, expectError } from "../../../helpers" +const Auth = require("../../../../src/enterprise/auth") +import { LoginCommand } from "../../../../src/commands/login" +import stripAnsi from "strip-ansi" + +function makeCommandParams(garden: TestGarden) { + const log = garden.log + return { + garden, + log, + headerLog: log, + footerLog: log, + args: {}, + opts: withDefaultGlobalOpts({}), + } +} + +describe("LoginCommand", () => { + let tmpDir: tmp.DirectoryResult + let projectConfig: ProjectConfig + const dummyDomain = "dummy-domain" + + before(async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await exec("git", ["init"], { cwd: tmpDir.path }) + + projectConfig = { + apiVersion: DEFAULT_API_VERSION, + kind: "Project", + name: "test", + path: tmpDir.path, + defaultEnvironment: "default", + dotIgnoreFiles: [], + environments: [{ name: "default", defaultNamespace, variables: {} }], + providers: [{ name: "test" }], + variables: {}, + } + }) + + beforeEach(async () => { + td.replace(Auth, "login", async () => "dummy-auth-token") + }) + + after(async () => { + await tmpDir.cleanup() + }) + + it("should log in if the project has a domain and an id", async () => { + const config = { ...projectConfig, domain: dummyDomain, id: "dummy-id" } + const garden = await TestGarden.factory(tmpDir.path, { config }) + const command = new LoginCommand() + await command.action(makeCommandParams(garden)) + }) + + it("should log in if the project has a domain but no id", async () => { + const config = { ...projectConfig, domain: dummyDomain } + const garden = await TestGarden.factory(tmpDir.path, { config }) + const command = new LoginCommand() + await command.action(makeCommandParams(garden)) + }) + + it("should throw if the project doesn't have a domain", async () => { + const garden = await TestGarden.factory(tmpDir.path, { config: projectConfig }) + const command = new LoginCommand() + await expectError( + () => command.action(makeCommandParams(garden)), + (err) => expect(stripAnsi(err.message)).to.match(/Your project configuration does not specify a domain/) + ) + }) +})