Operator sdkv1.4.2

[ArangoGutierrez]

Update the Operator SDK version used for the operator

currently NFD is under V0.18 this Patch moves it to V1.4.2

Note: this is the first of many patches, but taking into account the amount of change of this patch, other planned things like bug fixes, even typos, will be fixed in a following PR

[ArangoGutierrez]

/assign marquiz

[zvonkok]

We (@marquiz implemented it) introduced the master/worker split because we wanted to increase security so that workers cannot label the node. This patch introduces one common SA nfd-operator used by master and worker, which in the end means an nfd-worker will have the same privileges as the master, namely

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: nfd-operator
rules:
- apiGroups:
  - ""
  resources:
  - pods
  - nodes
  verbs:
  - get
  - patch
  - update 

Please keep the RBAC for master and worker separate. A worker does not need get,patch,update on the node. The worker should run with minimal privs.

[ArangoGutierrez]

We (@marquiz implemented it) introduced the master/worker split because we wanted to increase security so that workers cannot label the node. This patch introduces one common SA nfd-operator used by master and worker, which in the end means an nfd-worker will have the same privileges as the master, namely

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: nfd-operator
rules:
- apiGroups:
  - ""
  resources:
  - pods
  - nodes
  verbs:
  - get
  - patch
  - update 

Please keep the RBAC for master and worker separate. A worker does not need get,patch,update on the node. The worker should run with minimal privs.

Got it, working on it

[marquiz]

Please keep the RBAC for master and worker separate. A worker does not need get,patch,update on the node. The worker should run with minimal privs.

Got it, working on it

Yes. The worker does not currently need any privileges/access to the k8s control plane so no rbac configuration should be present for it.

[ArangoGutierrez]

@zvonkok @marquiz Ready for review again, RBAC is now master and worker

[ArangoGutierrez]

@marquiz @zvonkok PING

[ArangoGutierrez]

/honk

[k8s-ci-robot]

@ArangoGutierrez:

In response to this:

/honk

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.

[ArangoGutierrez]

/woof

[k8s-ci-robot]

@ArangoGutierrez:

In response to this:

/woof

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.

[zvonkok]

/joke

[k8s-ci-robot]

@zvonkok: Why is no one friends with Dracula? Because he's a pain in the neck.

In response to this:

/joke

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.

[zvonkok]

Ok looks good, but there are some files without 'newline' at EOF and some empty lines. Please check, after that, I will lgtm.

[zvonkok]

Otherwise most of the diff is based on the new operator-sdk layout and introduction of kustomize

[ArangoGutierrez]

Ok looks good, but there are some files without 'newline' at EOF and some empty lines. Please check, after that, I will lgtm.

for i in $(sed -ns '${/./F}' ./**/*.yaml); do echo "" >> $i; done

[ArangoGutierrez]

now rebase giving trouble again.....fixing

[ArangoGutierrez]

@marquiz ready for review PTAL
CC @zvonkok

[ArangoGutierrez]

Fixed the issue preventing the make image to succeed, should be ready for review now

[marquiz]

Still a few comments and open questions.

Please also drop the manager binary that appeared in the latest version of the PR

[marquiz]

Looks good, now!

One more nit: could you remove the trailing whitespace from the ends of lines, introduced by this patch. I spotted these files (but there might be others, too):

• Makefile
• build/assets/master/0100_service_account.yaml
• build/assets/master/0200_clusterrole.yaml
• config/certmanager/kustomizeconfig.yaml
• config/rbac/nodefeaturediscovery_editor_role.yaml
• config/samples/nfd.kubernetes.io_v1_nodefeaturediscovery.yaml

[mythi]

just a thought but I was wondering should the CRD be cluster scoped instead of Namespaced.

[ArangoGutierrez]

this is the same CRD , this patch intention is about the operator framework layout, changes to the CRD will come in a following PR

[mythi]

time for k8s.io/klog/v2?

[ArangoGutierrez]

same comment as the CRD, this patch is big enough like to introduce more changes, I am planning on enhancing logging and adding changes to the CRD but not in this patch that is big enough.
Thanks for the reviews Mythi :)

[ArangoGutierrez]

Looks good, now!

One more nit: could you remove the trailing whitespace from the ends of lines, introduced by this patch. I spotted these files (but there might be others, too):

* Makefile

* build/assets/master/0100_service_account.yaml

* build/assets/master/0200_clusterrole.yaml

* config/certmanager/kustomizeconfig.yaml

* config/rbac/nodefeaturediscovery_editor_role.yaml

* config/samples/nfd.kubernetes.io_v1_nodefeaturediscovery.yaml

@marquiz requestes files have been trimmed RFR

[marquiz]

@marquiz requestes files have been trimmed RFR

Are they? Didn't see any change on the trailing whitespace 🤔 I was referring to the trailing whitespace on the non-empty lines. I did see that now the empty lines from the end of those files were removed, though.

[marquiz]

Thanks @ArangoGutierrez, looks good! 🎉 I think we're done with this megalomanian PR 😉
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez, marquiz

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 [ArangoGutierrez,marquiz]

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