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

Parallel Reconciler: Create EventPolicies for Parallel #7984

Closed
creydr opened this issue Jun 10, 2024 · 7 comments · Fixed by #8112
Closed

Parallel Reconciler: Create EventPolicies for Parallel #7984

creydr opened this issue Jun 10, 2024 · 7 comments · Fixed by #8112
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Issues which should be fixed (post-triage)

Comments

@creydr
Copy link
Member

creydr commented Jun 10, 2024

Similar to a Sequence, the Parallel implementation uses Channels under the hood. This means that the Parallel

flowchart LR
    A[Service A] --> Parallel
    S1 --> X[Service B]
    S2 --> X[Service B]
    
    subgraph Parallel
    
    F1(Filter 1) --> S1(Service 1)
    F2(Filter 2) --> S2(Service 3)
    end
Loading

breaks down to something like

flowchart LR
    A[Service A] --> C0
    D2 -->|reply from Service 1| X[Service B]
    D4 -->|reply from Service 2| X

    subgraph Parallel

        C0(Channel 0)
        C0 -.-> D1(Dispatcher)
        D1 --> F1
        D1 -->|reply from Filter 1| C1
        subgraph "Subscription 1 with Subscriber: Filter 1, Reply: Channel 1, Channel: Channel 0"
            F1(Filter 1)
        end
        
        C1 -.-> D2
        D2(Dispatcher) --> S1
        subgraph "Subscription 2 with Subscriber: Service 1, Reply: Service B, Channel: Channel 1"
            C1(Channel 1)
            S1(Service 1)
        end

        C0 -.-> D3
        D3(Dispatcher) --> F2
        subgraph "Subscription 3 with Subscriber: Filter 2, Reply: Channel 2, Channel: Channel 0"
            F2(Filter 2)
        end

        D3 -->|reply from Filter 2| C2
        D4(Dispatcher) --> S2
        C2 -.-> D4
        subgraph "Subscription 4 with Subscriber: Service 2, Reply: Service B, Channel: Channel 2"
            C2(Channel 2)
            S2(Service 2)
        end

    end
Loading

Therefor we need to make sure we have the correct EventPolicies in place to not block requests to the underlying channel. So the Parallel reconciler should behave as described:

  • In case the authentication-oidc feature flag is set to enabled:
    • create EventPolicies like for the above example:
      • EventPolicy for Channel1:
        • .spec.ref: pointing to Channel1
        • .spec.from: OIDC identity of Subscription1. This means .spec.from is a ref to Subscription1
      • EventPolicy for Channel2:
        • .spec.ref: pointing to Channel2
        • .spec.from: OIDC identity of Subscription3. This means .spec.from is a ref to Subscription3
      • EventPolicy for Channel0:
        • This EventPolicy will only be created, if we have an EventPolicy for the Parallel in place (e.g. created by the user). This is because Channel0 represents the input channel of the Parallel and we would not be aware of the allowed subs. But as soon as an EventPolicy for the Parallel is in place, the Parallel reconciler would also create an EventPolicy for its input channel (Channel0 here) with the allowed subjects from the EventPolicy targeting the Parallel.
      • owner reference of the EventPolicies points to the Sequence, so that we have a lifecycle binding
  • In case the authentication-oidc feature flag is set to disabled:
    • clean up eventually existing EventPolicies which were created when authentication-oidc was enabled (e.g. by filtering on EventPolicies which have an owner reference to a Parallel)

Prerequisites:

Additional context:

Additional hints for new contributors before starting with this issue:

  1. When the issue has the Draft status, the issue is subject to change and thus should not be started to be worked on
  2. Make sure you've read and understood the CONTRIBUTING.md guidelines
  3. Make sure you're able to run Knative Eventing locally and run at least the unit tests.
  4. Feel free to raise any questions you have either directly here in the issue, in the #knative-eventing Slack channel or join the Eventing Workgroup Meeting
  5. When you feel comfortable with this issue, feel free to assign it to you (e.g. by commenting /assign). Please be aware that we might unassign you, if we don't see any progress from your side to give other contributors also a chance to work on this issue.
@creydr creydr added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Issues which should be fixed (post-triage) labels Jun 10, 2024
@swastik959
Copy link

/assign

@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 9, 2024

Hey @swastik959, are there anything I can help you with regarding understanding this issue?

@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 15, 2024

@swastik959 All the prerequisite for this issue has been met, and it is ready to be worked on.

@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 22, 2024

Are there anything I could help you with? @swastik959

If you're still working on this issue, please let me know within the next 24 hours. We understand that plans and priorities can change, and if you're no longer able to continue with this task, that's completely okay! In case I don't hear back from you in the next 24 hours, I'll unassign the issue from you. Of course, if you'd like to continue working on it later, you can always reassign it to yourself if it is still available.

@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 23, 2024

@rahulii: Thanks for reaching out! As we didn't see any progress on this, I think you can take this over if you want!
/unassign @swastik959
/assign @rahulii

@rahulii
Copy link
Contributor

rahulii commented Jul 25, 2024

hey @Leo6Leo , here Channel0 (in the above diagram) is an ingressChannel , right ?
for example in Parallel Status

ingressChannelStatus:
      channel:
        apiVersion: messaging.knative.dev/v1
        kind: InMemoryChannel
        name: odd-even-parallel-kn-parallel
        namespace: default
      ready:
        lastTransitionTime: null
        status: "True"
        type: Ready

this is Channel0, as per above diagram, correct ?

@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 25, 2024

@rahulii Hey Rahul, yes you are correct, Channel 0 in the above diagram is an ingresChannel. Hope that helps!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Issues which should be fixed (post-triage)
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants