fix(certs-v5): Removing references to SSL endpoints in the CLI (#1885)

* Removing references to SSL endpoints in the CLI * Fix test * Fix tests * Temp * remove unused feature * wip: fix tests * Stop checking for multipleSniEndpointsEnabled * Updated tests to work with Multi-SNI * remove addDomains * ensure no prompt * update test name * remove vestigial describe * simplify type function * cleanup info names * SNI is the only kind of endpoint * add delete test Co-authored-by: Jessie A. Young <jessie.young@salesforce.com> Co-authored-by: Michael Hale <mhale@heroku.com> Co-authored-by: Gary Clark <gary.clark@salesforce.com>
heroku · Aug 4, 2022 · 94c1d98 · 94c1d98
1 parent d064ef7
commit 94c1d98
Show file tree

Hide file tree

Showing 25 changed files with 151 additions and 1,459 deletions.
diff --git a/.gitignore b/.gitignore
@@ -11,3 +11,7 @@ node_modules
 /packages/cli/docs/*
 
 .vscode
+
+**/.DS_Store
+./idea
+
diff --git a/docs/certs.md b/docs/certs.md
@@ -49,7 +49,6 @@ OPTIONS
   -a, --app=app        (required) app to run command against
   -r, --remote=remote  git remote of app to use
   --bypass             bypass the trust chain completion step
-  --type=type          type to create, either 'sni' or 'endpoint'
 
 DESCRIPTION
   Note: certificates with PEM encoding are also valid

diff --git a/packages/apps/src/commands/domains/add.ts b/packages/apps/src/commands/domains/add.ts
@@ -4,7 +4,6 @@ import * as Heroku from '@heroku-cli/schema'
 import cli from 'cli-ux'
 import {prompt} from 'inquirer'
 import * as shellescape from 'shell-escape'
-import checkMultiSni from '../../lib/multiple-sni-feature'
 import waitForDomain from '../../lib/wait-for-domain'
 
 interface DomainCreatePayload {
@@ -75,8 +74,6 @@ export default class DomainsAdd extends Command {
     const {args, flags} = this.parse(DomainsAdd)
     const {hostname} = args
 
-    const multipleSniEndpointsEnabled = await checkMultiSni(this.heroku, flags.app)
-
     const domainCreatePayload: DomainCreatePayload = {
       hostname,
       sni_endpoint: null,
@@ -85,15 +82,12 @@ export default class DomainsAdd extends Command {
     let certs: Array<Heroku.SniEndpoint> = []
 
     cli.action.start(`Adding ${color.green(domainCreatePayload.hostname)} to ${color.app(flags.app)}`)
-    if (multipleSniEndpointsEnabled) {
-      // multiple SNI endpoints is enabled
-      if (flags.cert) {
-        domainCreatePayload.sni_endpoint = flags.cert
-      } else {
-        const {body} = await this.heroku.get<Array<Heroku.SniEndpoint>>(`/apps/${flags.app}/sni-endpoints`)
+    if (flags.cert) {
+      domainCreatePayload.sni_endpoint = flags.cert
+    } else {
+      const {body} = await this.heroku.get<Array<Heroku.SniEndpoint>>(`/apps/${flags.app}/sni-endpoints`)
 
-        certs = [...body]
-      }
+      certs = [...body]
     }
 
     if (certs.length > 1) {

diff --git a/packages/apps/src/commands/domains/index.ts b/packages/apps/src/commands/domains/index.ts
@@ -2,7 +2,6 @@ import {Command, flags} from '@heroku-cli/command'
 import * as Heroku from '@heroku-cli/schema'
 import cli from 'cli-ux'
 import * as Uri from 'urijs'
-import checkMultiSni from '../../lib/multiple-sni-feature'
 
 function isApexDomain(hostname: string) {
   if (hostname.includes('*')) return false
@@ -73,7 +72,6 @@ www.example.com  CNAME            www.example.herokudns.com
 
   async run() {
     const {flags} = this.parse(DomainsIndex)
-    const multipleSniEndpointsEnabled = await checkMultiSni(this.heroku, flags.app)
     const {body: domains} = await this.heroku.get<Array<Heroku.Domain>>(`/apps/${flags.app}/domains`)
     const herokuDomain = domains.find(domain => domain.kind === 'heroku')
     const customDomains = domains.filter(domain => domain.kind === 'custom')
@@ -86,7 +84,7 @@ www.example.com  CNAME            www.example.herokudns.com
       if (customDomains && customDomains.length > 0) {
         cli.log()
         cli.styledHeader(`${flags.app} Custom Domains`)
-        cli.table(customDomains, this.tableConfig(multipleSniEndpointsEnabled), {
+        cli.table(customDomains, this.tableConfig(true), {
           ...flags,
           'no-truncate': true,
         })

diff --git a/packages/apps/src/lib/multiple-sni-feature.ts b/packages/apps/src/lib/multiple-sni-feature.ts
diff --git a/packages/apps/test/commands/domains/add.test.ts b/packages/apps/test/commands/domains/add.test.ts
@@ -19,41 +19,6 @@ describe('domains:add', () => {
     status: 'pending',
   }
 
-  describe('adding a domain without the feature flag existing all', () => {
-    test
-    .stderr()
-    .nock('https://api.heroku.com', api => api
-    .get('/apps/myapp/features')
-    .reply(200, [])
-    .post('/apps/myapp/domains', {hostname: 'example.com', sni_endpoint: null})
-    .reply(200, domainsResponse),
-    )
-    .command(['domains:add', 'example.com', '--app', 'myapp'])
-    .it('adds the domain to the app', ctx => {
-      expect(ctx.stderr).to.contain('Adding example.com to myapp... done')
-    })
-  })
-
-  describe('adding a domain without the feature flag on (the old way)', () => {
-    test
-    .stderr()
-    .nock('https://api.heroku.com', api => api
-    .get('/apps/myapp/features')
-    .reply(200, [
-      {
-        name: 'allow-multiple-sni-endpoints',
-        enabled: false,
-      },
-    ])
-    .post('/apps/myapp/domains', {hostname: 'example.com', sni_endpoint: null})
-    .reply(200, domainsResponse),
-    )
-    .command(['domains:add', 'example.com', '--app', 'myapp'])
-    .it('adds the domain to the app', ctx => {
-      expect(ctx.stderr).to.contain('Adding example.com to myapp... done')
-    })
-  })
-
   describe('adding a domain to an app with multiple certs', () => {
     const domainsResponseWithEndpoint = {
       ...domainsResponse,
@@ -66,13 +31,6 @@ describe('domains:add', () => {
       test
       .stderr()
       .nock('https://api.heroku.com', api => api
-      .get('/apps/myapp/features')
-      .reply(200, [
-        {
-          name: 'allow-multiple-sni-endpoints',
-          enabled: true,
-        },
-      ])
       .post('/apps/myapp/domains', {
         hostname: 'example.com',
         sni_endpoint: 'my-cert',
@@ -114,13 +72,6 @@ describe('domains:add', () => {
         return Promise.resolve({cert: 'my-cert'})
       })
       .nock('https://api.heroku.com', api => api
-      .get('/apps/myapp/features')
-      .reply(200, [
-        {
-          name: 'allow-multiple-sni-endpoints',
-          enabled: true,
-        },
-      ])
       .post('/apps/myapp/domains', {
         hostname: 'example.com',
         sni_endpoint: 'my-cert',

diff --git a/packages/apps/test/commands/domains/index.test.ts b/packages/apps/test/commands/domains/index.test.ts
@@ -90,8 +90,6 @@ describe('domains', () => {
 
   test
   .nock('https://api.heroku.com', api => api
-  .get('/apps/myapp/features')
-  .reply(200, [])
   .get('/apps/myapp/domains')
   .reply(200, herokuOnlyDomainsResponse),
   )
@@ -105,8 +103,6 @@ describe('domains', () => {
 
   test
   .nock('https://api.heroku.com', api => api
-  .get('/apps/myapp/features')
-  .reply(200, [])
   .get('/apps/myapp/domains')
   .reply(200, herokuAndCustomDomainsResponse),
   )
@@ -124,13 +120,6 @@ describe('domains', () => {
 
   test
   .nock('https://api.heroku.com', api => api
-  .get('/apps/myapp/features')
-  .reply(200, [
-    {
-      name: 'allow-multiple-sni-endpoints',
-      enabled: true,
-    },
-  ])
   .get('/apps/myapp/domains')
   .reply(200, herokuDomainWithSniEndpoint),
   )

diff --git a/packages/certs-v5/README.md b/packages/certs-v5/README.md
@@ -46,7 +46,6 @@ OPTIONS
   -a, --app=app        (required) app to run command against
   -r, --remote=remote  git remote of app to use
   --bypass             bypass the trust chain completion step
-  --type=type          type to create, either 'sni' or 'endpoint'
 
 DESCRIPTION
   Note: certificates with PEM encoding are also valid

diff --git a/packages/certs-v5/commands/certs/add.js b/packages/certs-v5/commands/certs/add.js
@@ -13,8 +13,7 @@ let isWildcard = require('../../lib/is_wildcard.js')
 let isWildcardMatch = require('../../lib/is_wildcard_match.js')
 let getCertAndKey = require('../../lib/get_cert_and_key.js')
 let matchDomains = require('../../lib/match_domains.js')
-let { checkMultiSniFeature } = require('../../lib/features.js')
-let { waitForDomains, printDomains } = require('../../lib/domains')
+let { waitForDomains } = require('../../lib/domains')
 
 function Domains (domains) {
   this.domains = domains
@@ -26,31 +25,7 @@ function Domains (domains) {
 }
 
 async function getMeta(context, heroku) {
-  let type = context.flags.type
-
-  if (type) {
-    switch (type) {
-      case 'endpoint':
-        return endpoints.meta(context.app, 'ssl')
-      case 'sni':
-        return endpoints.meta(context.app, 'sni')
-      default:
-        error.exit(1, "Must pass --type with either 'endpoint' or 'sni'")
-    }
-  }
-
-  let [ hasSpace, hasAddon ] = await Promise.all([
-    endpoints.hasSpace(context.app, heroku),
-    endpoints.hasAddon(context.app, heroku)
-  ])
-
-  if (hasSpace && !context.canMultiSni) {
-    return endpoints.meta(context.app, 'ssl')
-  } else if (!hasAddon) {
-    return endpoints.meta(context.app, 'sni')
-  } else {
-    error.exit(1, "Must pass --type with either 'endpoint' or 'sni'")
-  }
+  return endpoints.meta(context.app, 'sni')
 }
 
 function hasMatch (certDomains, domain) {
@@ -82,107 +57,6 @@ async function getChoices(certDomains, newDomains, existingDomains, context) {
   }
 }
 
-async function addDomains(context, heroku, meta, cert) {
-  let certDomains = cert.ssl_cert.cert_domains
-
-  let apiDomains = await waitForDomains(context, heroku)
-
-  let existingDomains = []
-  let newDomains = []
-  let herokuDomains = []
-
-  certDomains.forEach(function (certDomain) {
-    let matches = findMatch(certDomain, apiDomains)
-    if (matches) {
-      if (matches.kind === 'heroku') {
-        herokuDomains.push(certDomain)
-      } else {
-        existingDomains.push(certDomain)
-      }
-    } else {
-      newDomains.push(certDomain)
-    }
-  })
-
-  if (herokuDomains.length > 0) {
-    cli.log()
-    cli.styledHeader('The following common names are for hosts that are managed by Heroku')
-    herokuDomains.forEach((domain) => cli.log(domain))
-  }
-
-  if (existingDomains.length > 0) {
-    cli.log()
-    cli.styledHeader('The following common names already have domain entries')
-    existingDomains.forEach((domain) => cli.log(domain))
-  }
-
-  let choices = await getChoices(certDomains, newDomains, existingDomains, context)
-  let domains
-
-  if (choices.length === 0) {
-    domains = new Domains([])
-  } else {
-    // Add a newline between the existing and adding messages
-    cli.console.error()
-
-    let promise = Promise.all(choices.map(function (certDomain) {
-      return heroku.request({
-        path: `/apps/${context.app}/domains`,
-        method: 'POST',
-        body: { 'hostname': certDomain }
-      }).catch(function (err) {
-        return { _hostname: certDomain, _failed: true, _err: err }
-      })
-    })).then(function (data) {
-      let domains = new Domains(data)
-      if (domains.hasFailed) {
-        throw domains
-      }
-      return domains
-    })
-
-    let label = choices.length > 1 ? 'domains' : 'domain'
-    let message = `Adding ${label} ${choices.map((choice) => cli.color.green(choice)).join(', ')} to ${cli.color.app(context.app)}`
-    domains = await cli.action(message, {}, promise).catch(function (err) {
-      if (err instanceof Domains) {
-        return err
-      }
-      throw err
-    })
-  }
-
-  if (domains.hasFailed) {
-    cli.log()
-    domains.failed.forEach(function (domain) {
-      cli.error(`An error was encountered when adding ${domain._hostname}`)
-      cli.error(domain._err)
-    })
-  }
-
-  cli.log()
-
-  let hasWildcard = _.some(certDomains, (certDomain) => isWildcard(certDomain))
-
-  let domainsTable = apiDomains.concat(domains.added)
-    .filter((domain) => domain.kind === 'custom')
-    .map(function (domain) {
-      let warning = null
-      if (hasWildcard && domain.hostname) {
-        if (!hasMatch(certDomains, domain.hostname)) {
-          warning = '! Does not match any domains on your SSL certificate'
-        }
-      }
-
-      return Object.assign({}, domain, { warning: warning })
-    })
-
-  printDomains(domainsTable, 'Your certificate has been added successfully.')
-
-  if (domains.hasFailed) {
-    error.exit(2)
-  }
-}
-
 async function configureDomains(context, heroku, meta, cert) {
   let certDomains = cert.ssl_cert.cert_domains
   let apiDomains = await waitForDomains(context, heroku)
@@ -212,10 +86,6 @@ async function configureDomains(context, heroku, meta, cert) {
 }
 
 async function run(context, heroku) {
-  let features = await heroku.get(`/apps/${context.app}/features`)
-  let canMultiSni = checkMultiSniFeature(features)
-  context.canMultiSni = canMultiSni
-
   let meta = await getMeta(context, heroku)
 
   let files = await getCertAndKey(context)
@@ -240,22 +110,10 @@ async function run(context, heroku) {
 
   certificateDetails(cert)
 
-  if (canMultiSni) {
-    await configureDomains(context, heroku, meta, cert)
-  } else {
-    await addDomains(context, heroku, meta, cert)
-  }
-
+  await configureDomains(context, heroku, meta, cert)
   displayWarnings(cert)
 }
 
-const CertTypeCompletion = {
-  skipCache: true,
-  options: (ctx) => {
-    return ['sni', 'endpoint']
-  }
-}
-
 module.exports = {
   topic: 'certs',
   command: 'add',
@@ -265,8 +123,7 @@ module.exports = {
     { name: 'KEY', optional: false }
   ],
   flags: [
-    { name: 'bypass', description: 'bypass the trust chain completion step', hasValue: false },
-    { name: 'type', description: "type to create, either 'sni' or 'endpoint'", hasValue: true, completion: CertTypeCompletion }
+    { name: 'bypass', description: 'bypass the trust chain completion step', hasValue: false }
   ],
   description: 'add an SSL certificate to an app',
   help: 'Note: certificates with PEM encoding are also valid',