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

cache.SelectorsByObject relies on internal struct #1697

Closed
amisevsk opened this issue Oct 15, 2021 · 1 comment · Fixed by #1703
Closed

cache.SelectorsByObject relies on internal struct #1697

amisevsk opened this issue Oct 15, 2021 · 1 comment · Fixed by #1703
Assignees

Comments

@amisevsk
Copy link

I'm trying out the SelectorsByObject cache option introduced in v0.9.0, and noticed a bit of a strange setup. The type for SelectorsByObject is

// SelectorsByObject associate a client.Object's GVK to a field/label selector.
type SelectorsByObject map[client.Object]internal.Selector

This results in SelectorsByObject values being semi-accessible, e.g. I can create objects with the internal type Selector in a map initialization:

selectors := cache.SelectorsByObject{
  &corev1.Pod{}: {
    Label: devworkspaceObjectSelector,
  },
}
internalSelector := selectors[&corev1.Pod{}]

but cannot add to SelectorsByObject directly (since I cannot create instances of internal.Selector):

selectors[&corev1.Service{}] = // can't instantiate object here

This results in a moderately strange workaround in my use case, where I want to include OpenShift Routes in SelectorsByObject only when my controller is running on OpenShift:

selectors := cache.SelectorsByObject{
  // ...
}

if isOpenShift {
  openShiftSelectors := cache.SelectorsByObject{
    &routev1.Route{}: {
      Label: myLabelSelector,
    },
  }
  for k, v := range openShiftSelectors {
    selectors[k] = v
  }
}

If the internal.Selector type is intended to be internal, then cache.SelectorsByObject should not use it. If it's intended to be used externally, it should not be in an internal package.

@FillZpp
Copy link
Contributor

FillZpp commented Oct 21, 2021

If the internal.Selector type is intended to be internal, then cache.SelectorsByObject should not use it. If it's intended to be used externally, it should not be in an internal package.

Indeed. I'd like to open a PR to fix it.
/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants