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

Should projectRef be a required field? #604

Closed
erikjoh opened this issue Feb 7, 2022 · 4 comments
Closed

Should projectRef be a required field? #604

erikjoh opened this issue Feb 7, 2022 · 4 comments
Labels
question Further information is requested

Comments

@erikjoh
Copy link

erikjoh commented Feb 7, 2022

I've noticed that several resources now require the projectRef field.

required:
- location
- projectRef

Whereas there are some resources that have but don't require the projectRef field.

projectRef:
description: The project that this resource belongs to.

Based on the discussion about projectRef in #349 , shouldn't the projectRef field be optional?
Since we have namespace level annotations for project I don't really see why it would be required in all cases.

For example now we must do:

apiVersion: networkservices.cnrm.cloud.google.com/v1beta1
kind: NetworkServicesMesh
metadata:
  name: mesh
  namespace: <project> # namespace that is annotated 'cnrm.cloud.google.com/project-id: <project>'
spec:
  location: global
  projectRef:
    external: projects/<project>

to not get a validation error (spec.projectRef: Required value).

Instead it would be nice to just have to do:

apiVersion: networkservices.cnrm.cloud.google.com/v1beta1
kind: NetworkServicesMesh
metadata:
  name: mesh
  namespace: <project> # namespace that is annotated 'cnrm.cloud.google.com/project-id: <project>'
spec:
  location: global

And let the field default based on the namespace annotation.

@erikjoh erikjoh added the question Further information is requested label Feb 7, 2022
@xiaobaitusi
Copy link
Contributor

Hi @erikjoh, thanks for posting the question.

In the long term, we are considering to deprecate the namespace-level cnrm.cloud.google.com/project-id annotation for a couple of reasons: 1) projectRef field in each resource should be the optimal approach to configure the project where the resource belongs to. Because the interface aligns consistently with the generic resource reference concept like topicRef. 2) the namespace defaulting logic requires the webhook to read the namespace object which causes some performance issue on upgrades or when a great number of resource being applied. We are suggested by GKE to minimize the outgoing calls from the webhook to the API server for the latency concern.

Therefore we suggest to specify the project info on the resource level using projectRef field or cnrm.cloud.google.com/project-id annotation. You can use kpt setter to specify the project for resources within the kpt package. See this kpt package example

Note that we are in the process of backfilling the projectRef field for existing resources. Newly released resources like NetworkServicesMesh will not support the cnrm.cloud.google.com/project-id annotation and only support the required projectRef field.

Is specifying the project info on the resource level maybe using kpt an acceptable alternative? Let us know what you think.

@erikjoh
Copy link
Author

erikjoh commented Feb 11, 2022

the namespace defaulting logic requires the webhook to read the namespace object which causes some performance issue on upgrades or when a great number of resource being applied

This is a strong enough reason to convince me projectRef should be required 😄
I think kpt would work for this.
Thanks for that detailed explanation!

@erikjoh erikjoh closed this as completed Feb 11, 2022
@zchenyu
Copy link

zchenyu commented Nov 22, 2022

@xiaobaitusi is there an issue we can track for this deprecation? Thanks!

@diviner524
Copy link
Collaborator

@zchenyu If you are referring to namespace-level cnrm.cloud.google.com/project-id. We will keep supporting this feature (although not recommended as explained in this thread) until the next major version.

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

No branches or pull requests

4 participants