Skip to content

Commit

Permalink
Revamp config var lookup to make it more robust
Browse files Browse the repository at this point in the history 
Currently, if the attachment name is lowercase, the config vars
will still be uppercase, and :tail and :write fail in ugly ways.
  • Loading branch information
Maciek Sakrejda committed Aug 4, 2017
1 parent 93871fc commit 2c66c9d
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 20 deletions.
3 changes: 2 additions & 1 deletion commands/topics_tail.js
View file
Expand Up @@ -21,7 +21,8 @@ function * tail (context, heroku) {
}

let appConfig = yield heroku.get(`/apps/${context.app}/config-vars`)
let attachment = yield heroku.get(`/apps/${context.app}/addon-attachments/${addon.name}`)
let attachment = yield heroku.get(`/apps/${context.app}/addon-attachments/${addon.name}`,
{ headers: { 'accept-inclusion': 'config_vars' } })
let config = clusterConfig(attachment, appConfig)
let consumer = new kafka.SimpleConsumer({
idleTimeout: IDLE_TIMEOUT,
Expand Down
3 changes: 2 additions & 1 deletion commands/topics_write.js
View file
Expand Up @@ -20,7 +20,8 @@ function * write (context, heroku) {
}

let appConfig = yield heroku.get(`/apps/${context.app}/config-vars`)
let attachment = yield heroku.get(`/apps/${context.app}/addon-attachments/${addon.name}`)
let attachment = yield heroku.get(`/apps/${context.app}/addon-attachments/${addon.name}`,
{ headers: { 'accept-inclusion': 'config_vars' } })
let config = clusterConfig(attachment, appConfig)

let producer = new kafka.Producer({
Expand Down
28 changes: 22 additions & 6 deletions lib/shared.js
View file
Expand Up @@ -12,15 +12,31 @@ function deprecated (cmd, newCmd, newTopic) {

function clusterConfig (attachment, config) {
if (!attachment) {
return null
throw new Error('Could not find add-on')
}

const findVar = (suffix, required = true) => {
const configKey = attachment.config_vars.find(v => v.endsWith('_' + suffix))
if (!configKey) {
if (required) {
throw new Error(`Could not find ${suffix} for ${attachment.name} on ${attachment.app.name}`)
} else {
return undefined
}
}
const configVal = config[configKey]
if (!configVal && required) {
throw new Error(`Config value ${suffix} for ${attachment.name} on ${attachment.app.name} is empty`)
}
return configVal
}

return {
url: config[attachment.name + '_URL'],
trustedCert: config[attachment.name + 'TRUSTED_CERT'],
clientCert: config[attachment.name + '_CLIENT_CERT'],
clientCertKey: config[attachment.name + '_CLIENT_CERT_KEY'],
prefix: config[attachment.name + '_PREFIX']
url: findVar('URL'),
trustedCert: findVar('TRUSTED_CERT'),
clientCert: findVar('CLIENT_CERT'),
clientCertKey: findVar('CLIENT_CERT_KEY'),
prefix: findVar('PREFIX', false)
}
}

Expand Down
11 changes: 6 additions & 5 deletions test/commands/topics_tail_test.js
View file
Expand Up @@ -53,6 +53,7 @@ describe('kafka:topics:tail', () => {
KAFKA_CLIENT_CERT: 'hunter3',
KAFKA_CLIENT_CERT_KEY: 'hunter4'
}
let stockConfigVars = Object.keys(config)

beforeEach(() => {
planName = 'heroku-kafka:beta-standard-2'
Expand Down Expand Up @@ -87,7 +88,7 @@ describe('kafka:topics:tail', () => {
it('warns and exits with an error if it cannot connect', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })
consumer.init = () => { throw new Error('oh snap') }

return cmd.run({app: 'myapp', args: { TOPIC: 'topic-1' }})
Expand All @@ -102,7 +103,7 @@ describe('kafka:topics:tail', () => {
it('warns and exits with an error if it cannot subscribe', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })
consumer.subscribe = () => { throw new Error('oh snap') }

return cmd.run({app: 'myapp', args: { TOPIC: 'topic-1' }})
Expand All @@ -117,7 +118,7 @@ describe('kafka:topics:tail', () => {
it('tails a topic and prints the results', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })

consumer.subscribe = (topic, callback) => {
callback([
Expand All @@ -141,7 +142,7 @@ describe('kafka:topics:tail', () => {
}, config)
api.get('/apps/myapp/config-vars').reply(200, configWithPrefix)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars.concat('KAFKA_PREFIX'), app: { name: 'sushi' } })

consumer.subscribe = (topic, callback) => {
expect(topic).to.equal('nile-1234.topic-1')
Expand All @@ -166,7 +167,7 @@ describe('kafka:topics:tail', () => {
}, config)
api.get('/apps/myapp/config-vars').reply(200, configWithPrefix)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars.concat('KAFKA_PREFIX'), app: { name: 'sushi' } })

consumer.subscribe = (topic, callback) => {
expect(topic).to.equal('nile-1234.topic-1')
Expand Down
15 changes: 8 additions & 7 deletions test/commands/topics_write_test.js
View file
Expand Up @@ -55,6 +55,7 @@ describe('kafka:topics:write', () => {
KAFKA_CLIENT_CERT: 'hunter3',
KAFKA_CLIENT_CERT_KEY: 'hunter4'
}
let stockConfigVars = Object.keys(config)

beforeEach(() => {
planName = 'heroku-kafka:beta-standard-2'
Expand Down Expand Up @@ -85,7 +86,7 @@ describe('kafka:topics:write', () => {
it('warns and exits with an error if it cannot connect', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })
producer.init = () => { throw new Error('oh snap') }

return expectExit(1, cmd.run({app: 'myapp', args: { TOPIC: 'topic-1' }}))
Expand All @@ -96,7 +97,7 @@ describe('kafka:topics:write', () => {
it('warns and exits with an error if it cannot send the message', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })
producer.send = () => { throw new Error('oh snap') }

return expectExit(1, cmd.run({app: 'myapp', args: { TOPIC: 'topic-1' }, flags: {}}))
Expand All @@ -107,7 +108,7 @@ describe('kafka:topics:write', () => {
it('sends a message', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })

producer.send = (payload) => {
expect(payload.topic).to.equal('topic-1')
Expand All @@ -132,7 +133,7 @@ describe('kafka:topics:write', () => {
withPrefixConfig.KAFKA_PREFIX = 'nile-1234.'
api.get('/apps/myapp/config-vars').reply(200, withPrefixConfig)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars.concat('KAFKA_PREFIX'), app: { name: 'sushi' } })

producer.send = (payload) => {
expect(payload.topic).to.equal('nile-1234.topic-1')
Expand All @@ -157,7 +158,7 @@ describe('kafka:topics:write', () => {
withPrefixConfig.KAFKA_PREFIX = 'nile-1234.'
api.get('/apps/myapp/config-vars').reply(200, withPrefixConfig)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars.concat('KAFKA_PREFIX'), app: { name: 'sushi' } })

producer.send = (payload) => {
expect(payload.topic).to.equal('nile-1234.topic-1')
Expand All @@ -179,7 +180,7 @@ describe('kafka:topics:write', () => {
it('uses given partition if specified', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })

producer.send = (payload) => {
expect(payload.topic).to.equal('topic-1')
Expand All @@ -201,7 +202,7 @@ describe('kafka:topics:write', () => {
it('uses given message key if specified', () => {
api.get('/apps/myapp/config-vars').reply(200, config)
api.get('/apps/myapp/addon-attachments/kafka-1')
.reply(200, { name: 'KAFKA' })
.reply(200, { name: 'KAFKA', config_vars: stockConfigVars, app: { name: 'sushi' } })

producer.send = (payload) => {
expect(payload.topic).to.equal('topic-1')
Expand Down

0 comments on commit 2c66c9d

Please sign in to comment.