GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources
https://cloud.google.com/config-connector/docs/overview
Apache License 2.0
884 stars 217 forks source link

services.serviceusage.cnrm.cloud.google.com missing "spec" field #246

Open snuggie12 opened 4 years ago

snuggie12 commented 4 years ago

Describe the bug services.serviceusage.cnrm.cloud.google.com does not have a "spec" field. This conflicts when creating one using FutureObject because it requires a field called spec.object.spec but if you try to create a service with one the kcc webhook will tell you that unknown fields are not allowed.

https://github.com/GoogleCloudPlatform/k8s-config-connector/issues/223#issuecomment-652582506 mentions a potential solution which would establish a spec field. I actually really need #223 resolved as well so perhaps this could kill two birds with one stone.

It is probably also worth it to check other kcc resources to see if they are missing a spec field and fix those as well.

ConfigConnector Version 1.15.0

To Reproduce

  1. Attempt to create a services.serviceusage.cnrm.cloud.google.com with a field spec: {}. Will get an error about not allowing unknown fields.
  2. Attempt to create a services.serviceusage.cnrm.cloud.google.com without a spec field but using a FutureObject. Will get an error saying spec.object.spec is required.
  3. Attempt to create a services.serviceusage.cnrm.cloud.google.com with a spec field using a FutureObject. Will get an error saying that the object cannot be found.
AlexBulankou commented 4 years ago

@snuggie12 , thanks for reporting! Can you please elaborate on your scenario: why are you trying to create a serviceusage resource using FutureObject? To your question about #223, this feature is planned for Q3 (ETA end of Sep. 2020), but I think your main issue is that you would like resources have spec field to be "FutureObject friendly".

snuggie12 commented 4 years ago

@AlexBulankou I was using a FutureObject due to the fact the project ID is not yet known at the time.

To your last point, yes, all KCC resources should be KCO friendly.

AlexBulankou commented 4 years ago

@snuggie12 , to clarify your scenario, why is the project ID now known at this time? Are you creating a project using KCC ?

snuggie12 commented 4 years ago

@AlexBulankou That's correct, I'm creating a folder, followed by a project, followed by these services.

AlexBulankou commented 4 years ago

Thanks @snuggie12 . Can you please share the config templates, including Project config and ServiceUsage config?

AlexBulankou commented 4 years ago

@snuggie12 , following up on this. Could you please include your full scenario configs in this bug, so we can understand it better end to end - your folder config, project config and service config. Thank you!

snuggie12 commented 4 years ago

@AlexBulankou Sorry took me a second to get back to you. I couldn't figure out why you needed the files and then it dawned on me that the services don't need number references for projects and the helm chart can generate the name-based project ID instead. That's when I ran into #135 which I think is going to be a problem for a single namespace managing multiple projects.

Anyways, here are the files in their original forms minus some obvious edits like PII. kcc246.tar.gz

AlexBulankou commented 4 years ago

Thanks @snuggie12 , yes this was exactly the reason for my question: as project ID is known beforehand and not generated I could not understand why you needed to use FutureObject and FieldReference. However your last response (and your comment # Could switch this to a regular object since helm has the name-based ID in the code clarifies it.

Regarding #135: I believe, @kibbles-n-bytes was verifying whether it was still the issue Regarding FutureObject requiring spec.object.spec. We'll follow up separately with the Orchestration component authors.

kibbles-n-bytes commented 4 years ago

Hey @snuggie12 , at the moment we do not guarantee all resources have a spec field; the only case where resources have one is when there are configuration options. Service enablement doesn't have any configuration, and thus is intended not to have a spec. I've reached out to the authors of the FutureObject so that they are aware of this scenario. Note that we are working to expose a configuration field for all our resources that allows for explicitly specifying resource IDs in the scenario that Kubernetes naming has conflicting restrictions compared with the underlying Cloud API, so eventually all resources will in fact have a spec.

As for the issue with #135 : The scenario here is very specifically that the cloudresourcemanager.googleapis.com service must be enabled on the project Config Connector's service account lives in in order to enable services. It should only be relevant during initial bootstrapping. After that, Config Connector will be able to create projects, enable services, and create resources in those projects fully declaratively.

snuggie12 commented 4 years ago

@kibbles-n-bytes can you elaborate on that? I see two potential scenarios. Let's say given two projects A which already exists and has kcc installed and the GSA has org-wide permissions to enable gcpservices and project B which doesn't exist yet:

  1. The scenario you described is when the kcc from A tries to enable a gcpservice for B after the project is created? And it's a catch 22 since you need to first enable that gcpservice before you can enable other gcpservices?

  2. I think I read that if your GSA is in project B this isn't a problem? If true, does IAM come enabled? Could I create a GSA in project B with the permission to enable gcpservices and the GSA in project B could enable cloudresourcemanager.googleapis.com from kcc (as well as any others?)

kibbles-n-bytes commented 4 years ago

It's the opposite. If KCC's GSA lives in project A, then project A is the one that needs the Cloud Resource Manager service enabled as a bootstrapping measure. From there, KCC can create project B and enable the service and everything else without needing any additional manual steps.