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

fix workload and identity filters #1109

Merged
merged 2 commits into from
Jul 6, 2023
Merged

fix workload and identity filters #1109

merged 2 commits into from
Jul 6, 2023

Conversation

kaworu
Copy link
Member

@kaworu kaworu commented Jul 6, 2023

See commits

@kaworu
observe: fix --from-workload and --to-workload
e623a58 
Before this patch, --{from,to}-workload would not work as expected. The
workload filter was added to only one side of the left/right filter, and
thus would not be taken into account:

    % hubble observe --from-pod cilium --to-workload app --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"]}'
        - '{"source_pod":["cilium"],"destination_workload":[{"name":"app"}]}'

The workload filter is added only to the allowlist's right/destination
filter, and since the allowlist's left/source filter is a superset of
the right filter, the resulting allowlist is equivalent to:

    % hubble observe --from-pod cilium --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"]}'

Another example where the combined flag uses both left and right
filters:

    % hubble observe --pod cilium --to-workload app --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"]}'
        - '{"destination_pod":["cilium"],"destination_workload":[{"name":"app"}]}'

The right filter will never match (assuming cilium is not part of the
app workload) and the left filter is again equivalent to the one we get
without `--to-workload app`.

The --from-workload filter had the same bug, but was added the only the
left filter instead of the right filter.

This patch ensure that --{from,to}-workload filters are added on both
sides, and adds a regression test case. After this patch we get:

    % hubble observe --from-pod cilium --to-workload app --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"],"destination_workload":[{"name":"app"}]}'

    % hubble observe --pod cilium --to-workload app --print-raw-filters
    allowlist:
    - '{"source_pod":["cilium"],"destination_workload":[{"name":"app"}]}'
    - '{"destination_pod":["cilium"],"destination_workload":[{"name":"app"}]}'

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
@kaworu
observe: fix --from-identity and --to-identity
955437a 
Before this patch, --{from,to}-identity would not work as expected. The
identity filter was added to only one side of the left/right filter, and
thus would not be taken into account:

    % hubble observe --from-pod cilium --to-identity world --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"]}'
        - '{"source_pod":["cilium"],"destination_identity":[2]}'

The identity filter is added only to the allowlist's right/destination
filter, and since the allowlist's left/source filter is a superset of
the right filter, the resulting allowlist is equivalent to:

    % hubble observe --from-pod cilium --print-raw-filters
    allowlist:
        - '{"source_pod":["cilium"]}'

Another example where the combined flag uses both left and right
filters:

    % hubble observe --pod cilium --to-identity world --print-raw-filters
    allowlist:
    - '{"source_pod":["cilium"]}'
    - '{"destination_pod":["cilium"],"destination_identity":[2]}'

The right filter will never match and the left filter is again
equivalent to the one we get without `--to-identity world`.

The --from-identity filter had the same bug, but was added the only the
left filter instead of the right filter.

This patch ensure that --{from,to}-identity filters are added on both
sides, and adds a regression test case. After this patch we get:

    % hubble observe --from-pod cilium --to-identity world --print-raw-filters
    allowlist:
    - '{"source_pod":["cilium"],"destination_identity":[2]}'

    % hubble observe --pod cilium --to-identity world --print-raw-filters
    allowlist:
    - '{"source_pod":["cilium"],"destination_identity":[2]}'
    - '{"destination_pod":["cilium"],"destination_identity":[2]}'

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
@kaworu kaworu added 🐛 kind/bug This is a bug in the Hubble logic. ⌨️ area/cli Impacts the command line interface of any command in the repository. priority/release-blocker This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Hubble. needs-backport/0.11 This PR needs backporting to the v0.11 branch labels Jul 6, 2023
@kaworu kaworu requested a review from a team as a code owner July 6, 2023 10:13
@kaworu kaworu requested review from lambdanis and removed request for a team July 6, 2023 10:13
@kaworu kaworu merged commit b421e17 into master Jul 6, 2023
5 checks passed
@kaworu kaworu deleted the pr/kaworu/fix-workload-and-identity-filters branch July 6, 2023 10:42
@kaworu kaworu mentioned this pull request Jul 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandro gandro gandro approved these changes

@lambdanis lambdanis Awaiting requested review from lambdanis lambdanis was automatically assigned from cilium/sig-hubble

Assignees
No one assigned
Labels
⌨️ area/cli Impacts the command line interface of any command in the repository. 🐛 kind/bug This is a bug in the Hubble logic. needs-backport/0.11 This PR needs backporting to the v0.11 branch priority/release-blocker This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Hubble.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kaworu @gandro