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.

Sign up for GitHub

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

Jump to bottom

docs(security): updates content for securing kafka access #10071

Merged
merged 2 commits into from
May 11, 2024
Merged

docs(security): updates content for securing kafka access #10071

merged 2 commits into from
May 11, 2024

Conversation

PaulRMellor
Copy link
Contributor

@PaulRMellor PaulRMellor commented May 7, 2024 •
edited
Loading

Documentation

Updates from review and edit of the content related to security to make it easier to understand and find the relevant information.

  • Removes repeated content, including for the following types of content
    • Certificate creation (present in a couple of places)
    • Configuring Kafka brokers and users - now present in a single example
  • Restructuring:
    • Setting up client access to a Kafka cluster now has no content related to securing access
    • Setting up secure client access example moves to securing access section
    • Network policies concepts moved to the network policies procedure
    • OAuth 2.0 moves into a separate section
    • Moves user quotas into separate section for better visibility
  • Cleanup: the following files have been removed to reduce redundancy and repetition (content absorbed or removed if replicated elsewhere)
    • Security options for Kafka (assembly-securing-kafka-brokers.adoc) -- not required
    • Securing access to Kafka brokers (assembly-securing-kafka.adoc) -- content moved
    • Securing user access to Kafka (proc-configuring-secure-kafka-user.adoc) -- content in example
    • Securing Kafka brokers (proc-securing-kafka.adoc) -- content in example

NOTE: OAUth content is subject to a separate review

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

@PaulRMellor PaulRMellor added this to the 0.41.0 milestone May 7, 2024
@PaulRMellor PaulRMellor requested review from scholzj and ppatierno May 7, 2024 15:10
@PaulRMellor PaulRMellor self-assigned this May 7, 2024
scholzj
scholzj reviewed May 7, 2024
View reviewed changes
documentation/assemblies/security/assembly-securing-access.adoc Outdated Show resolved Hide resolved
documentation/assemblies/security/assembly-securing-kafka-clients.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-setup-external-clients.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-setup-external-clients.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-setup-external-clients.adoc Outdated
Comment on lines 32 to 33
Authorization options for Kafka brokers include `simple`, `OAuth`, `OPA`, or `custom`.
When enabled, authorization is applied to all enabled listeners.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned -> these are the server sides only. Not the supported types in the KAfkaUSer. We should make it clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think changing the order resolves this now.

documentation/modules/overview/con-configuration-points-listeners.adoc
However, you can customize this configuration using the `useServiceDnsDomain` property.
Consider using a `cluster-ip` type listener if routing through the headless service isn't feasible or if you require a custom access mechanism, such as when integrating with specific Ingress controllers or the Kubernetes Gateway API.

External listeners:: Use external listener types to connect clients outside a Kubernetes cluster.
+
* `nodeport` to use ports on Kubernetes nodes
* `loadbalancer` to use loadbalancer services
* `ingress` to use Kubernetes `Ingress` and the {NginxIngressController} (Kubernetes only)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, you can install the Nginx Ingress controller for example on oPenShift as well. So I think the focus should be on the controller rather on Kubernetes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We say "(Kubernernetes only)" in a few places when referring to listener types as we don't include the ingress procedure downstream.

documentation/modules/overview/con-configuration-points-listeners.adoc Outdated Show resolved Hide resolved
documentation/modules/security/con-configuring-client-quotas.adoc Outdated Show resolved Hide resolved
documentation/modules/security/con-securing-client-authentication.adoc Outdated Show resolved Hide resolved
documentation/modules/security/con-securing-kafka-authentication.adoc Outdated Show resolved Hide resolved
@scholzj scholzj modified the milestones: 0.41.0, 0.42.0 May 7, 2024
@PaulRMellor
Copy link
Contributor Author

Thanks for the review @scholzj . I addressed all the comments, but this one: #10071 (comment)

As mentioned in the reply, we use the "(Kubernetes only)" in a few places because of downstream doc where the ingress procedure is left out.

ppatierno
ppatierno approved these changes May 11, 2024
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@scholzj scholzj merged commit 3905a2a into strimzi:main May 11, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scholzj scholzj scholzj approved these changes

@ppatierno ppatierno ppatierno approved these changes

Assignees

@PaulRMellor PaulRMellor

Labels
documentation
Projects
None yet
Milestone
0.42.0
Development

Successfully merging this pull request may close these issues.
3 participants
@PaulRMellor @scholzj @ppatierno