feat: add support for Kafka secure connection with certificates X509 #70

derberg · 2021-05-20T11:55:57Z

Fixes #66 and adds support for Kafka security using X509 security scheme.

enabled kafka-secure
added new securityScheme parameter that one need to use to provide the name of the scheme to use for code generation
move some logic to filters as doing some stuff with nunjucks made my eyes bleed 😄
in case of security of X509 type the following additional code goes to app/index.js

const fs = require('fs')

serverConfig.ssl.ca = fs.readFileSync('ca.pem');
serverConfig.ssl.key = fs.readFileSync('service.key');
serverConfig.ssl.cert = fs.readFileSync('service.cert');

How I tested:

created account and kafka instance on Aiven
took spec from https://github.com/aiven/thingum-industries/blob/main/asyncapi.yaml and just changed the server URL and that is it
generated server by passing additional new param -p securityScheme=creds
got certs from Aiven
put certs files in the root of server generated using my changes
installed generated server
run server NODE_ENV=production npm start
Got the following:

 PUB  Will eventually publish to factorysensor
Thingum Industries Sensors 1.0.0 is ready! 

🔗  Kafka adapter is connected!

I assume it works, but did not really test sending any events. @lornajane could you have a look, review PR and check if it works as expected. To use my template changes, do not run ag against @asyncapi/nodejs-template but https://github.com/derberg/nodejs-template#x509

lornajane · 2021-05-20T19:32:08Z

Amazing! I tried it on the branch and the only thing I needed to change was to add an SSL entry to my config:

      ssl:
        rejectUnauthorized: true

Without it, there's no ssl property to write to. I'm not sure though if I've done something wrong in my aysncapi file to cause this to be missing!

derberg · 2021-05-20T20:24:17Z

@lornajane I think you are not passing the NODE_ENV like this NODE_ENV=production npm start

below setting is there in the config for production environment:

ssl:
        rejectUnauthorized: true

it is not set to true by default because default !== production and therefor should be flexible aka non secure for testing

lornajane · 2021-05-25T20:25:58Z

OK yes, that made it work! But then why am I specifying the server at generate time if I am going to specify at run time anyway? And must the servers be named like the sections in the configuration file? Production / development etc? Can I have a server named "Fred"?

derberg · 2021-05-26T12:14:56Z

@lornajane you can even have a server called Wilma if you really want 😄

I think always 2 cases should be supported:

if you have servers in the AsyncAPI document anyway, then why not reusing it
still you should be able to provide different server URL at runtime

what you see in common.yml is not 1:1 related to the servers section from AsyncAPI document. It is only related to https://github.com/motdotla/dotenv and the 12factor, to have config in env variables, easy to overwrite, so NODE_ENV=production only specifies that what is in default must be combined with what is in production. When you have a look on the template, we do not read server URL or anything like that but just apply some production-specific config for kafka only https://github.com/asyncapi/nodejs-template/blob/master/template/config/common.yml#L58.

Does it make sense?

derberg · 2021-05-26T12:15:36Z

@lornajane maybe I should add something more to readme, so code change suggestions would be highly appreciated 🙏🏼

derberg · 2021-06-02T08:04:22Z

@lornajane thoughts?

lornajane · 2021-06-13T14:40:35Z

I think a note in the README (maybe in the generated README?) on using the environment variable to set which server config to combine with the default would be useful.

But the PR change here is definitely an improvement and should be merged when we can. Thankyou!

derberg · 2021-06-14T14:59:36Z

@lornajane I pushed some readme updates. Need approval from another maintainer and we can merge

fmvilas

How difficult would it be to make the location of the certificates configurable?

README.md

Co-authored-by: Fran Méndez <fmvilas@gmail.com>

derberg · 2021-06-14T16:32:05Z

How difficult would it be to make the location of the certificates configurable?

@fmvilas depends, if all would be expected in single directory, then we just need to add one more param, so the default would be root, otherwise provided dirname. If custom directory per file, then the complicated part would be to agree on params names and how they should be provided. Sounds like better would be to get a use case first really

smoya · 2021-06-14T16:32:20Z

README.md

 ## How to use the template

 This template must be used with the AsyncAPI Generator. You can find all available options [here](https://github.com/asyncapi/generator/).

+In case you use X509 security and need to provide certificates, place them in the root of generated server with the following names: `ca.pem`, `service.cert`, `service.key`. Since you can have multiple different security schemes, to use the one of X509 type, you need to pass the name of the scheme like this: `-p securityScheme=SCHEME_NAME`.


As a suggestion: Could we provide the absolute path instead? I think it would be easier for the users and simplifies a lot the file handling. Most of this paragraph will be then removed.

I just realized @fmvilas wrote some similar suggestion at #70 (review)

option to provide custom location for the files is valid, but I don't get how will it affect this paragraph, other then extend it more 😄

For me this is simplified not only in terms of documentation but improved UX.

Suggested change

In case you use X509 security and need to provide certificates, place them in the root of generated server with the following names: `ca.pem`, `service.cert`, `service.key`. Since you can have multiple different security schemes, to use the one of X509 type, you need to pass the name of the scheme like this: `-p securityScheme=SCHEME_NAME`.

In case you use X509 security and need to provide certificates, specify their location at BLA BLA. Since you can have multiple different security schemes, to use the one of X509 type, you need to pass the name of the scheme like this: `-p securityScheme=SCHEME_NAME`.

for me best DX is defaults and later config. Description updated

smoya · 2021-06-14T16:35:45Z

template/src/api/index.js

+{%- if params.securityScheme and (asyncapi.server(params.server).protocol() === 'kafka' or asyncapi.server(params.server).protocol() === 'kafka-secure') and asyncapi.components().securityScheme(params.securityScheme).type() === 'X509' %}
+const fs = require('fs')
+
+serverConfig.ssl.ca = fs.readFileSync('ca.pem');


What if the file does not exist? is somehow erroring?

added proper try/catch, thanks!

fmvilas · 2021-06-14T17:19:04Z

Sounds like better would be to get a use case first really

It's pretty common to have certificates in a system directory instead of the app directory, so they can be shared. Actually, beyond proof-of-concepts, it's not very common to find certificate files in the same dir as the code. I think having the ability to simply setup the root directory where to look for certificates would be good enough to start.

derberg · 2021-06-15T07:37:59Z

It's pretty common to have certificates in a system directory instead of the app directory, so they can be shared. Actually, beyond proof-of-concepts, it's not very common to find certificate files in the same dir as the code. I think having the ability to simply setup the root directory where to look for certificates would be good enough to start.

makes sense to me, will work on it

…o x509

derberg · 2021-06-15T09:57:56Z

@fmvilas @smoya

certFilesDir added, docs extended

smoya

LGTM 👏 💯 🚀

derberg · 2021-06-22T06:47:29Z

@fmvilas kind reminder 😉

sonarqubecloud · 2021-06-22T06:47:53Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

fmvilas

Great stuff! 🚀

asyncapi-bot · 2021-06-22T09:27:40Z

🎉 This PR is included in version 0.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

derberg added 2 commits May 20, 2021 13:42

feat: add support for Kafka secure connection with certificates X509

a1b6291

better readme

aa1f6df

derberg added 2 commits June 14, 2021 13:48

Merge branch 'master' into x509

3c8c2a1

add more clarity to readme

40ace98

fmvilas requested changes Jun 14, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

c254bfc

Co-authored-by: Fran Méndez <fmvilas@gmail.com>

smoya reviewed Jun 14, 2021

View reviewed changes

derberg added 2 commits June 15, 2021 11:52

added certFilesDir param, docs and error handling

b52759c

Merge branch 'x509' of https://github.com/derberg/nodejs-template int…

a9a9f65

…o x509

derberg requested review from fmvilas and smoya June 15, 2021 09:58

smoya approved these changes Jun 15, 2021

View reviewed changes

Merge branch 'master' into x509

df90da7

fmvilas approved these changes Jun 22, 2021

View reviewed changes

derberg merged commit 12c8435 into asyncapi:master Jun 22, 2021

derberg deleted the x509 branch June 22, 2021 09:25

asyncapi-bot added the released label Jun 22, 2021

github-actions bot approved these changes Jun 22, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for Kafka secure connection with certificates X509 #70

feat: add support for Kafka secure connection with certificates X509 #70

derberg commented May 20, 2021

lornajane commented May 20, 2021

derberg commented May 20, 2021

lornajane commented May 25, 2021

derberg commented May 26, 2021

derberg commented May 26, 2021

derberg commented Jun 2, 2021

lornajane commented Jun 13, 2021 •

edited

Loading

derberg commented Jun 14, 2021

fmvilas left a comment

derberg commented Jun 14, 2021

smoya Jun 14, 2021

smoya Jun 14, 2021

derberg Jun 15, 2021

smoya Jun 15, 2021 •

edited

Loading

derberg Jun 15, 2021

smoya Jun 14, 2021

derberg Jun 15, 2021

fmvilas commented Jun 14, 2021

derberg commented Jun 15, 2021

derberg commented Jun 15, 2021

smoya left a comment

derberg commented Jun 22, 2021

sonarqubecloud bot commented Jun 22, 2021

fmvilas left a comment

asyncapi-bot commented Jun 22, 2021

	In case you use X509 security and need to provide certificates, place them in the root of generated server with the following names: `ca.pem`, `service.cert`, `service.key`. Since you can have multiple different security schemes, to use the one of X509 type, you need to pass the name of the scheme like this: `-p securityScheme=SCHEME_NAME`.
	In case you use X509 security and need to provide certificates, specify their location at BLA BLA. Since you can have multiple different security schemes, to use the one of X509 type, you need to pass the name of the scheme like this: `-p securityScheme=SCHEME_NAME`.

feat: add support for Kafka secure connection with certificates X509 #70

feat: add support for Kafka secure connection with certificates X509 #70

Conversation

derberg commented May 20, 2021

lornajane commented May 20, 2021

derberg commented May 20, 2021

lornajane commented May 25, 2021

derberg commented May 26, 2021

derberg commented May 26, 2021

derberg commented Jun 2, 2021

lornajane commented Jun 13, 2021 • edited Loading

derberg commented Jun 14, 2021

fmvilas left a comment

Choose a reason for hiding this comment

derberg commented Jun 14, 2021

smoya Jun 14, 2021

Choose a reason for hiding this comment

smoya Jun 14, 2021

Choose a reason for hiding this comment

derberg Jun 15, 2021

Choose a reason for hiding this comment

smoya Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

derberg Jun 15, 2021

Choose a reason for hiding this comment

smoya Jun 14, 2021

Choose a reason for hiding this comment

derberg Jun 15, 2021

Choose a reason for hiding this comment

fmvilas commented Jun 14, 2021

derberg commented Jun 15, 2021

derberg commented Jun 15, 2021

smoya left a comment

Choose a reason for hiding this comment

derberg commented Jun 22, 2021

sonarqubecloud bot commented Jun 22, 2021

fmvilas left a comment

Choose a reason for hiding this comment

asyncapi-bot commented Jun 22, 2021

lornajane commented Jun 13, 2021 •

edited

Loading

smoya Jun 15, 2021 •

edited

Loading