I think this should be !ok to early out if we don't find the deprecated api in the set of CRs

I think that is actually a check that’s saying “if this resource is already covered by one that’s listed in the CSV as something the operator uses, then that’s a more reliable check, so skip this check”

This might be something we want to add a comment for since it isn't immediately obvious why we'd be bailing out when we find an RBAC rule for a resource within a deprecated API.

Taking a look at this, I think it could really go either way. The intention behind this section is exactly what @joelanford mentioned.

I think we could either add a comment or adopt the recommendation by @oceanc80 and remove the continue statement within the block.

i.e go from:

for _, res := range rule.Resources {
	if _, ok := resInCsvCrds[res]; ok {
		continue
	}
       warnIfDeprecated(schema.GroupResource{Group: apiGroup, Resource: res}, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
}

to:

for _, res := range rule.Resources {
	if _, ok := resInCsvCrds[res]; !ok {
            warnIfDeprecated(schema.GroupResource{Group: apiGroup, Resource: res}, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
	}
}

Technically saves us a line of code and if it feels more readable that way I am happy to change it

Same question here. Can we make this a map[string]struct{} (or use sets.String from k8s.io/apimachinery)?

After adding maps to resolve your previous comment, I ended up removing this function definition as the map used by warnIfDeprecated has the schema.GroupResource as a key. If the given schema.GroupResource doesn't match any of the keys it doesn't warn.

I think the only downside to having removed this function is that now it always runs the loop to create a schema.GroupResource and verify it instead of only if the group has a deprecated api. If this is something we are concerned about I can re-implement this function.

@joelanford @oceanc80 WDYT?