Change namespace creation for osd-logging

[petrkotas]

What type of PR is this?

BUG

What this PR does / why we need it?

Moved namespace creation from osd-logging to be create only. Additional resources are created in patches.

This is change related to bug:

• https://issues.redhat.com/browse/OSD-25576

Which Jira/Github issue(s) this PR fixes?

Fixes # https://issues.redhat.com/browse/OSD-25576

Special notes for your reviewer:

Pre-checks (if applicable):

• Tested latest changes against a cluster

• Included documentation changes with PR

• If this is a new object that is not intended for the FedRAMP environment (if unsure, please reach out to team FedRAMP), please exclude it with:

  matchExpressions:
  - key: api.openshift.com/fedramp
    operator: NotIn
    values: ["true"]

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: petrkotas
Once this PR has been reviewed and has the lgtm label, please assign fahlmant for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[petrkotas]

@jewzaam and @2uasimojo would you please be able to review this? Thank you.

[2uasimojo]

I'm obviously not going to be qualified to review this for content, but for readability, consider using yaml's multiline string syntax:

Suggested change

	patch: '{"rules":[{"apiGroups":[""],"resources":["events","namespaces","persistentvolumeclaims","persistentvolumes","pods","pods/log"],"verbs":["list","get","watch"]},{"apiGroups":[""],"resources":["secrets"],"verbs":["*"]},{"apiGroups":["logging.openshift.io"],"resources":["clusterloggings"],"verbs":["create","delete","deletecollection","get","list","patch","update","watch"]},{"apiGroups":["operators.coreos.com"],"resources":["subscriptions","clusterserviceversions"],"verbs":["*"]},{"apiGroups":["operators.coreos.com"],"resources":["installplans"],"verbs":["update"]},{"apiGroups":[""],"resources":["persistentvolumeclaims"],"verbs":["*"]},{"apiGroups":["apps","extensions"],"resources":["daemonsets"],"verbs":["get","list","patch","update","watch"]}]}'
	patch: |
	{
	"rules": [
	{
	"apiGroups": [
	""
	],
	"resources": [
	"events",
	"namespaces",
	"persistentvolumeclaims",
	"persistentvolumes",
	"pods",
	"pods/log"
	],
	"verbs": [
	"list",
	"get",
	"watch"
	]
	},
	{
	"apiGroups": [
	""
	],
	"resources": [
	"secrets"
	],
	"verbs": [
	"*"
	]
	},
	{
	"apiGroups": [
	"logging.openshift.io"
	],
	"resources": [
	"clusterloggings"
	],
	"verbs": [
	"create",
	"delete",
	"deletecollection",
	"get",
	"list",
	"patch",
	"update",
	"watch"
	]
	},
	{
	"apiGroups": [
	"operators.coreos.com"
	],
	"resources": [
	"subscriptions",
	"clusterserviceversions"
	],
	"verbs": [
	"*"
	]
	},
	{
	"apiGroups": [
	"operators.coreos.com"
	],
	"resources": [
	"installplans"
	],
	"verbs": [
	"update"
	]
	},
	{
	"apiGroups": [
	""
	],
	"resources": [
	"persistentvolumeclaims"
	],
	"verbs": [
	"*"
	]
	},
	{
	"apiGroups": [
	"apps",
	"extensions"
	],
	"resources": [
	"daemonsets"
	],
	"verbs": [
	"get",
	"list",
	"patch",
	"update",
	"watch"
	]
	}
	]
	}

Similar below.

FYI I generated the above guts via:

efried@efried-thinkpadp16vgen1:~/go/src/github.com/openshift/hive$ echo '{"rules":[{"apiGroups":[""],"resources":["events","namespaces","persistentvolumeclaims","persistentvolumes","pods","pods/log"],"verbs":["list","get","watch"]},{"apiGroups":[""],"resources":["secrets"],"verbs":["*"]},{"apiGroups":["logging.openshift.io"],"resources":["clusterloggings"],"verbs":["create","delete","deletecollection","get","list","patch","update","watch"]},{"apiGroups":["operators.coreos.com"],"resources":["subscriptions","clusterserviceversions"],"verbs":["*"]},{"apiGroups":["operators.coreos.com"],"resources":["installplans"],"verbs":["update"]},{"apiGroups":[""],"resources":["persistentvolumeclaims"],"verbs":["*"]},{"apiGroups":["apps","extensions"],"resources":["daemonsets"],"verbs":["get","list","patch","update","watch"]}]}' | jq | sed 's/^/                          /'

• jq helpfully formats the JSON
• Using sed to prefix each line with the correct number of spaces (jq --indent unfortunately doesn't do what we need here).

[2uasimojo]

I'm not sure we actually want to use a patch for this Role, which based on its name is intended to be SREP-owned. Is there a situation where you would want to preserve some existing -- presumably customer admin-injected -- rules in this Role?

IIUC the general intent behind this change is to make sure we're preserving customer settings on objects they're supposed to be allowed to modify -- like labels on OpenShift namespaces.

[2uasimojo]

Duplicated line

Suggested change

	namespace: openshift-logging
	namespace: openshift-logging
	namespace: openshift-logging

[2uasimojo]

This file rename doesn't look intentional. Similar with other .bak renames below.

(There's generally no need to check in "backed up" files, as the files are always available from earlier commits.)

[jewzaam]

/hold

This changes from creation of resources to patching only. Implication is any place where this is applied for the first time for a cluster it will do nothing because there is nothing to patch. JIRA is the right place to sort out the plan for how this is applied and implications to existing or new clusters. Not clear right now what the requirements are.

[openshift-ci]

@petrkotas: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

[2uasimojo]

Syntactically this seems sane at a glance. IIUC there's still some discussion around whether SREP's management of this namespace is correct/desired at a high level.