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

chore: filter bug #651

Merged
merged 6 commits into from
Mar 14, 2024
Merged

chore: filter bug #651

merged 6 commits into from
Mar 14, 2024

Conversation

cmwylie19
Copy link
Collaborator

Description

In Binding Filters, labels do not have to have values, they only have to have keys. In the event that there is not value, there is an implication that the value is "" (empty string).

This scenario was overlooked in the checkOverlap function which checks if the binding's filters/annotations have overlap with the objects.

This was causing an error in UDS-Core as a pod was being ignored that contained the same key as the binding but a different value.

export function checkOverlap(record1: Record<string, string>, record2: Record<string, string>) {
  if (Object.keys(record1).length === 0) {
    return true;
  }
  for (const key in record1) {
    if (
      Object.prototype.hasOwnProperty.call(record1, key) &&
      Object.prototype.hasOwnProperty.call(record2, key) &&
      record1[key] === record2[key] // VALUES MUST BE THE SAME
    ) {
      return true;
    }
  }
  return false;
}

For Example:

When(a.ConfigMap)
  .IsCreated()
  .WithLabel("chuck-norris")
  .Mutate(async change => {
    // Try/catch is not needed as a response object will always be returned
    const response = await fetch<TheChuckNorrisJoke>(
      "https://api.chucknorris.io/jokes/random?category=dev",
    );

image

image

Related Issue

Fixes #

Relates to #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
@cmwylie19
Copy link
Collaborator Author

Need to accommodate for AND, when both labels should match. This functionality currently does not

When(a.Pod)
  .IsUpdated()
  .WithLabel("batch.kubernetes.io/job-name")
  .WithLabel("service.istio.io/canonical-name")

btlghrants
btlghrants previously approved these changes Mar 14, 2024
Copy link
Collaborator

@btlghrants btlghrants left a comment

Choose a reason for hiding this comment

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

Seems pretty legit.

src/lib/helpers.test.ts Outdated Show resolved Hide resolved
src/lib/helpers.test.ts Outdated Show resolved Hide resolved
cmwylie19 and others added 2 commits March 14, 2024 11:57
good looks!

Co-authored-by: Barrett <81570928+btlghrants@users.noreply.github.com>
Co-authored-by: Barrett <81570928+btlghrants@users.noreply.github.com>
Copy link
Collaborator

@btlghrants btlghrants left a comment

Choose a reason for hiding this comment

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

K!

@cmwylie19 cmwylie19 merged commit ce1257b into main Mar 14, 2024
11 checks passed
@cmwylie19 cmwylie19 deleted the filter_bug branch March 14, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants