Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add security mechanism info to Kafka bindings for server objects #466

Closed
dalelane opened this issue Nov 26, 2020 · 7 comments
Closed

Add security mechanism info to Kafka bindings for server objects #466

dalelane opened this issue Nov 26, 2020 · 7 comments

Comments

@dalelane
Copy link
Collaborator

dalelane commented Nov 26, 2020

Is your feature request related to a problem? Please describe.
Kafka has several values that are essential to know when preparing to connect to a secured Kafka cluster, such as sasl.mechanism and security.protocol. Adding these to the Kafka binding for server objects would allow these to be documented.

Can't it be tackled using specification extensions?
It could be captured in extensions.

e.g.

asyncapi: '2.0.0'
...
servers:
  production:
    url: localhost:9092
    protocol: kafka-secure
    security:
    - saslScramCreds: []
...
components:
  securitySchemes:
    saslScramCreds:
      type: userPassword
      x-mykafka-sasl-mechanism: 'SCRAM-SHA-256'
      x-mykafka-security-protocol: 'SASL_SSL'

However I think this is a common enough request that there would be value in doing it in a consistent way. In particular, it would enable code generation to set up the right Kafka client properties.

Describe the solution you'd like
Strawman suggestion:

asyncapi: '2.0.0'
...
servers:
  production:
    url: localhost:9092
    protocol: kafka-secure
    security:
    - saslScramCreds: []
    bindings:
      kafka:
        saslMechanism: 'SCRAM-SHA-256'
        securityProtocol: 'SASL_SSL'
...
components:
  securitySchemes:
    saslScramCreds:
      type: userPassword

Describe alternatives you've considered
Fran Méndez suggested:

We should make it part of the security schemes, it's not only for Kafka after all.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Jan 26, 2021
@fmvilas fmvilas added this to the AsyncAPI specification 2.1.0 milestone Jan 31, 2021
dalelane added a commit to dalelane/bindings that referenced this issue Feb 24, 2021
There are a huge number of Kafka client configs we could add to
server bindings, but I'm skeptical of the value of just copying
them all across. Most client config values have sensible
defaults that are suitable in most cases.
cf. https://kafka.apache.org/documentation/#consumerconfigs

Two values are essential for code generation with secured Kafka
clusters, however - identifying the security mechanism and
protocol to use. This commit adds these two most commonly-used
parameters to the Kafka server bindings. I suggest we start with
these and gauge usage before starting to add many more, as these
are enough to enable code generation for secured Kafka clusters.

Closes: asyncapi/spec#466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
@magicmatatjahu
Copy link
Member

magicmatatjahu commented Mar 10, 2021

I wonder if adding another security mechanism to the AsyncAPI JSON Schema it makes sense in the long term. I mean looking at this table asyncapi/bindings#56 (comment) I have this problem that we may forget something and will have to tweak it in the AsyncAPI scheme and publish new version etc. Would treating securityScheme as extension cause problems in long term? I mean extension like:

# extension
id: saslSecurityScheme
type: securityScheme # new type of extension
title: ...
...

# spec
components:
  securitySchemes:
    usingSaslSecurityScheme
       type: saslSecurityScheme
       ...

I am aware that the above schema is not fully correct, but I hope that the idea itself is understandable.

@fmvilas @derberg @jonaslagoni @smoya @dalelane Any thoughts?

@fmvilas
Copy link
Member

fmvilas commented Mar 10, 2021

we may forget something and will have to tweak it in the AsyncAPI scheme and publish new version

That's not a problem. Publishing a new version of the JSON Schema definition of the spec is not the same as publishing a new version of the spec. Remember, the spec is only the Markdown file.

@dalelane
Copy link
Collaborator Author

@fmvilas @derberg A few PRs ready for this now:

#502
Update the spec markdown description

asyncapi/spec-json-schemas#53
Update the JSON Schema definition

asyncapi/parser-js#289
Update the JS Parser to use the new JSON Schema definition

@fmvilas fmvilas removed this from the Next specification version milestone May 12, 2021
fmvilas pushed a commit that referenced this issue May 28, 2021
* feat: add SASL security scheme types for use by Kafka specs

As discussed in asyncapi/bindings#56 this
adds additional security scheme types. The motivation for adding
them is to enable description of secured Kafka clusters, however
the security protocols and mechanisms being added are not unique
to Kafka, so this commit adds them as generic security schemes so
they could be used by other protocols as well.

Contributes to: #466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>

* chore: update for grammar

Moved the "and" to the end of the list.

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>

* docs: additional SASL documentation

Contributes to: #466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
fmvilas pushed a commit to asyncapi/spec-json-schemas that referenced this issue May 28, 2021
* chore: Start new schema file for 2.1.0

Copy of 2.0.0.json

Contributes to: asyncapi/spec#466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>

* feat: Add SASL mechanisms to the schema

Contributes to: asyncapi/spec#466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>

* chore: Update schema version number

Contributes to: asyncapi/spec#466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.1.0-2021-06-release.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

fmvilas added a commit to asyncapi/parser-js that referenced this issue May 28, 2021
* feat: update version for updated spec

Contributes to: asyncapi/spec#466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>

* Use @asyncapi/spec@2.8.0-2021-06-release.1

Co-authored-by: Fran Mendez <fmvilas@gmail.com>
fmvilas pushed a commit that referenced this issue Jun 1, 2021
Main motivation of this was to provide a place to demonstrate
the new Kafka-specific security schemes introduced in
AsyncAPI 2.1.0. It was also a chance to remove the Kafka
bindings from the otherwise-MQTT sample.

The need for this was discussed in
#502

Contributes to: #466

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.2.0-2021-06-release.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@jonaslagoni
Copy link
Member

I guess we can close this right? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants