doc: add internal image proposal

[robszumski]

Description of the change:
This PR adds a proposal for marking a subset of CRDs as "internal", meaning they are not for end-user consumption or manipulation.

Motivation for the change:
Many Operators have internal concepts that are confusing to end-users when they show up in user interfaces or CLIs like operatorhub.io. This proposal solidifies this concept into a standard convention that others can follow to hide these CRDs and downstream user interfaces by hide them if deemed necessary for their user base.

[joelanford]

@robszumski Along these same lines, there is also a distinction between workload/application CRDs and data-only CRDs. Such a distinction would make running the SDK scorecard more reliable and may be helpful metadata for a user.

Thoughts on that? And if you agree, would it make sense to include this concept in this proposal as well?

[sbose78]

+1 !

We were looking to see if there's a good way to hide some of the CRDs which the users don't need to , on the UI ( and other places where we want to hide out details/abstractions ) :

This sounds like a good idea.

[sbose78]

Let's make it apps.kubernetes.io/internal-object:true ? :)

[robszumski]

Merging these two comments together would leave us with:

kind: CustomResourceDefinition
apiVersion: apiextensions.k8s.io/v1beta1
metadata:
  name: hivetables.metering.openshift.io
  annotations:
    apps.kubernetes.io/internal-object:true
    apps.kubernetes.io/data-object:true

I don't see any issues with that and can get the proposal updated. WDYT @joelanford @sbose78?

[sbose78]

That'll be good, @robszumski .

[robszumski]

PTAL again, updated title, filename and added the new annotations.

[ecordell]

Unfortunately, we can't use the k8s namespace without upstream approval.

Can we use operators.operatorframework.io prefix like we do for our bundles?

operators.operatorframework.io.bundle.mediatype.v1: "registry+v1" for example

[shawn-hurley]

apps.operatorframework.io?

[openshift-ci-robot]

@robszumski: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-console-olm	4cc0432	link	/test e2e-aws-console-olm
ci/prow/e2e-aws-olm	4cc0432	link	/test e2e-aws-olm

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[bbrowning]

Would this annotation be appropriate to hide things that should not show up to end-users in things like the Developer Catalog but instead can only be created by cluster admins? Or will this annotation also hide it from cluster admins in screens outside of just the Developer Catalog?

In my case, I want admins to see the option to create a KnativeServing CR from their Installed Operators screen. But I don't want developers to be offered to create a KnativeServing CR from their Developer Catalog. We have RBAC that limits who can create them, but last I checked the Dev Catalog doesn't inspect the RBAC rules for CRs when deciding what to show.

[christianvogt]

@robszumski what about supporting these annotations on the CSV. Looks like the console will have an issue:
openshift/console#3170 (comment)

[robszumski]

@christianvogt Are you thinking it goes on to spec.customresourcedefinitions.owned? No issues on that for me other than it's now a deeper OLM spec change not an optional field to be read (or not).

[spadgett]

The problem here for console is that normal users can't list CRDs, so there's no way to console read the annotation. We currently look at spec.customresourcedefinitions.owned in the CSV. So this would either need to be a new field there or perhaps an annotation on the CSV itself.

[christianvogt]

@sbose78 had suggested an annotation on the CSV which was originally a whitelist array of CRDs kinds I believe. A agree though that a blacklist approach is better.

[sbose78]

My initial suggestion was to have :

apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
metadata:
  annotations:
    apps.operatorframework.io/primary-objects: [....] 

• This avoids the need to make a deeper OLM CSV schema change
• The typical RBAC design for consumption of OLM lets non-admin users read CSVs compared to reading CRDs. Hence, having it on the CSV might help a bigger audience consume this information.
• Downside: Doesn't make it OLM agnostic.

What do you folks think ?

[robszumski]

@sbose78 Turning that into a blacklist seems like a good solution to me

[christianvogt]

An annotation on CSV whose value is an array of names spec.owned.customresourcedefinitions.name.
Blacklist and renamed @sbose78's proposal to align with the previous naming:
operators.operatorframework.io/internal-objects: [...]

[sbose78]

@christianvogt & @robszumski :

Sounds good.

• the blacklist, looks good to me 👍
• use of spec.owned.customresourcedefinitions.name in the array provides uniqueness, so 👍 .

apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
metadata:
  annotations:
    apps.operatorframework.io/internal-objects: [....] 

[christianvogt]

@robszumski @sbose78
Dev console is going to go ahead with aligning on this proposal noted above.

[sbose78]

Sounds good.

[abhinandan13jan]

FYI openshift/console#3518

[abhinandan13jan]

@sbose78 @christianvogt Please let me know if this is to be changed

@christianvogt & @robszumski :

Sounds good.

• the blacklist, looks good to me +1
• use of spec.owned.customresourcedefinitions.name in the array provides uniqueness, so +1 .

apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
metadata:
  annotations:
    apps.operatorframework.io/internal-objects: [....] 

As part of CR the annotation considered is operators.operatorframework.io/internal-objects and not apps.operatorframework.io/internal-objects mentioned in the quoted comment.

[christianvogt]

@sbose78 @robszumski
Siince the annotation must be a string. I just want to confirm that this will be a stringified JSON array value: eg '["foo","bar"]'

[ecordell]

@robszumski do you have time to fix this proposal up to match the decisions made in the comments here?

[matzew]

Is this still planed to be able to hide some CRDs ?

[ecordell]

@matzew This is already implemented in console

[ecordell]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ecordell, robszumski

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ecordell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ecordell]

doc-only, merging without tests.