Skip to content

Conversation

pierDipi
Copy link
Member

Based on #95

  • Document authorization policies for MTChannelBasedBroker

pierDipi added 2 commits June 12, 2023 15:01
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@netlify
Copy link

netlify bot commented Jun 13, 2023

Deploy Preview for jazzy-shortbread-5f62b7 ready!

Name Link
🔨 Latest commit 75961a7
🔍 Latest deploy log https://app.netlify.com/sites/jazzy-shortbread-5f62b7/deploys/64917191ada58600082acb89
😎 Deploy Preview https://deploy-preview-96--jazzy-shortbread-5f62b7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci
Copy link

openshift-ci bot commented Jun 13, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pierDipi

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi pierDipi force-pushed the SRVKE-1419_sm-docs-tech-preview-authorization branch from f7a9f0d to 0cbd988 Compare June 13, 2023 11:45
namespace: knative-eventing
spec: { } <1>
----
<1> Disallow any operations to every workload that is part of the service mesh in the `knative-eventing` namespace.
Copy link
Member

Choose a reason for hiding this comment

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

I like the flow, of denying anything and than open it up

paths: [ "/authz-tests/*" ] <3>
----
<1> Allow workloads in the `authz-tests` namespace
<2> To post events to Knative brokers in the `authz-tests` namespace.
Copy link
Member

Choose a reason for hiding this comment

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

should we say that this is only true for the MT-Channel-based broker? (instead of generic Knative Brokers):

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

$ oc apply -f <filename>
----

. Create a `AuthorizationPolicy` in the `knative-eventing` namespace to allow `mt-broker-ingress` in
Copy link
Member

Choose a reason for hiding this comment

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

this all seem to render as bold?

Copy link
Member Author

Choose a reason for hiding this comment

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

post events to a Knative Broker with class MTChannelBasedBroker.

. Create a `AuthorizationPolicy` in the `knative-eventing` namespace to allow pods
in the `authz-tests` namespace to send events to Knative Brokers in the same `authz-tests` namespace:
Copy link
Member

Choose a reason for hiding this comment

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

to allow pods in the authz-tests namespace

should we clarifiy this is for all pods? E.g. not just the container source image/pod?

----

. Create a `AuthorizationPolicy` in the `knative-eventing` namespace to allow `mt-broker-ingress` in
the `knative-eventing` namespace to post events to the `InMemoryChannel` dispatcher:
Copy link
Member

Choose a reason for hiding this comment

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

I may overthink here.
But perhaps some may wonder on why the ingress posts to the dispatcher?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I can add a little sentence

Comment on lines +262 to +263
. Create a `AuthorizationPolicy` in the `knative-eventing` namespace to allow `imc-dispatcher` in
the `knative-eventing` namespace to post events to the `mt-broker-filter`:
Copy link
Member

Choose a reason for hiding this comment

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

I guess same as above. but I am not really sure if this doc needs to explain all in depth :-)


.Verification

You can verify that the events were sent to the Knative event sink by looking at the message dumper function logs.
Copy link
Member

Choose a reason for hiding this comment

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

I really like the E2E example, and the detailed configuration example here for patching/configuring all the components needed, in order to make this work

Copy link
Contributor

@mgencur mgencur left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me! I have just some minor nits. And I haven't tested this myself.

:description: Access control for Knative Brokers with {SMProductName}

By default, every workload is allowed to send events to a Knative Broker, with {SMProductName}, we can
apply policies to control who can post events to Knative brokers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
apply policies to control who can post events to Knative brokers.
apply policies to control who can post events to Knative Brokers.


+
we have denied access to every workload to the knative-eventing namespace, which disallows the
`ContainerSource` `heartbeat-source-mt-channel-based-broker` to send events to the Knative `Broker`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`ContainerSource` `heartbeat-source-mt-channel-based-broker` to send events to the Knative `Broker`
ContainerSource `heartbeat-source-mt-channel-based-broker` to send events to the Knative Broker

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this but highlighting also the type (ContainerSource) in the same way as the name looks strange.

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi pierDipi force-pushed the SRVKE-1419_sm-docs-tech-preview-authorization branch from 64bc4e3 to 75961a7 Compare June 20, 2023 09:29
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pierDipi
Copy link
Member Author

pierDipi commented Oct 2, 2023

Done in #97

@pierDipi pierDipi closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants