From 63eb7f63f6f23b4b6dc347c5767f550ada633161 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Tue, 12 Nov 2024 19:17:29 +0000 Subject: [PATCH] address comments --- apis/bigqueryconnection/v1alpha1/connection_types.go | 3 +-- .../bigqueryconnection/v1beta1/connection_reference.go | 9 +++++---- apis/bigqueryconnection/v1beta1/connection_types.go | 3 +-- ...tions.bigqueryconnection.cnrm.cloud.google.com.yaml | 10 ++-------- .../v1beta1/bigqueryconnectionconnection_types.go | 2 +- .../direct/bigqueryconnection/connection_controller.go | 8 ++++---- .../bigqueryconnection/bigqueryconnectionconnection.md | 2 +- 7 files changed, 15 insertions(+), 22 deletions(-) diff --git a/apis/bigqueryconnection/v1alpha1/connection_types.go b/apis/bigqueryconnection/v1alpha1/connection_types.go index 7bd89b0848..b665863921 100644 --- a/apis/bigqueryconnection/v1alpha1/connection_types.go +++ b/apis/bigqueryconnection/v1alpha1/connection_types.go @@ -38,10 +38,9 @@ type Parent struct { type BigQueryConnectionConnectionSpec struct { Parent `json:",inline"` - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ResourceID field is immutable" // Immutable. Optional. // The BigQuery Connection ID used for resource creation or acquisition. - // For creation: If specified, this value is used as the connection ID. If not provided, a UUID is generated and stored in the `status.ExternalRef` field. + // For creation: If specified, this value is used as the connection ID. If not provided, a UUID will be generated and assigned as the connection ID. // For acquisition: This field must be provided to identify the connection resource to acquire. ResourceID *string `json:"resourceID,omitempty"` diff --git a/apis/bigqueryconnection/v1beta1/connection_reference.go b/apis/bigqueryconnection/v1beta1/connection_reference.go index fad12b056f..74baa3a7f7 100644 --- a/apis/bigqueryconnection/v1beta1/connection_reference.go +++ b/apis/bigqueryconnection/v1beta1/connection_reference.go @@ -108,15 +108,16 @@ func (r *BigQueryConnectionConnectionRef) Parent() (string, error) { return "", fmt.Errorf("BigQueryConnectionConnectionRef not normalized to External form or not created from `New()`") } -func (r *BigQueryConnectionConnectionRef) ConnectionID() (string, error) { +// ConnectionID returns the connection ID, a boolean indicating whether the connection ID is specified by user (or generated by service), and an error. +func (r *BigQueryConnectionConnectionRef) ConnectionID() (string, bool, error) { if r.External != "" { _, _, id, err := parseExternal(r.External) if err != nil { - return "", err + return "", false, err } - return id, nil + return id, id != "", nil } - return "", fmt.Errorf("BigQueryConnectionConnectionRef not normalized to External form or not created from `New()`") + return "", false, fmt.Errorf("BigQueryConnectionConnectionRef not normalized to External form or not created from `New()`") } // NormalizedExternal provision the "External" value. diff --git a/apis/bigqueryconnection/v1beta1/connection_types.go b/apis/bigqueryconnection/v1beta1/connection_types.go index c2a9d8c2e6..3abf3880ce 100644 --- a/apis/bigqueryconnection/v1beta1/connection_types.go +++ b/apis/bigqueryconnection/v1beta1/connection_types.go @@ -38,10 +38,9 @@ type Parent struct { type BigQueryConnectionConnectionSpec struct { Parent `json:",inline"` - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ResourceID field is immutable" // Immutable. Optional. // The BigQuery Connection ID used for resource creation or acquisition. - // For creation: If specified, this value is used as the connection ID. If not provided, a UUID is generated and stored in the `status.ExternalRef` field. + // For creation: If specified, this value is used as the connection ID. If not provided, a UUID will be generated and assigned as the connection ID. // For acquisition: This field must be provided to identify the connection resource to acquire. ResourceID *string `json:"resourceID,omitempty"` diff --git a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_bigqueryconnectionconnections.bigqueryconnection.cnrm.cloud.google.com.yaml b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_bigqueryconnectionconnections.bigqueryconnection.cnrm.cloud.google.com.yaml index de80d209f0..44793ad73c 100644 --- a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_bigqueryconnectionconnections.bigqueryconnection.cnrm.cloud.google.com.yaml +++ b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_bigqueryconnectionconnections.bigqueryconnection.cnrm.cloud.google.com.yaml @@ -308,13 +308,10 @@ spec: description: 'Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID - is generated and stored in the `status.ExternalRef` field. For acquisition: + will be generated and assigned as the connection ID. For acquisition: This field must be provided to identify the connection resource to acquire.' type: string - x-kubernetes-validations: - - message: ResourceID field is immutable - rule: self == oldSelf spark: description: Spark properties. properties: @@ -808,13 +805,10 @@ spec: description: 'Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID - is generated and stored in the `status.ExternalRef` field. For acquisition: + will be generated and assigned as the connection ID. For acquisition: This field must be provided to identify the connection resource to acquire.' type: string - x-kubernetes-validations: - - message: ResourceID field is immutable - rule: self == oldSelf spark: description: Spark properties. properties: diff --git a/pkg/clients/generated/apis/bigqueryconnection/v1beta1/bigqueryconnectionconnection_types.go b/pkg/clients/generated/apis/bigqueryconnection/v1beta1/bigqueryconnectionconnection_types.go index dbf965d38d..8c7c77e744 100644 --- a/pkg/clients/generated/apis/bigqueryconnection/v1beta1/bigqueryconnectionconnection_types.go +++ b/pkg/clients/generated/apis/bigqueryconnection/v1beta1/bigqueryconnectionconnection_types.go @@ -189,7 +189,7 @@ type BigQueryConnectionConnectionSpec struct { /* The Project that this resource belongs to. */ ProjectRef v1alpha1.ResourceRef `json:"projectRef"` - /* Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID is generated and stored in the `status.ExternalRef` field. For acquisition: This field must be provided to identify the connection resource to acquire. */ + /* Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID will be generated and assigned as the connection ID. For acquisition: This field must be provided to identify the connection resource to acquire. */ // +optional ResourceID *string `json:"resourceID,omitempty"` diff --git a/pkg/controller/direct/bigqueryconnection/connection_controller.go b/pkg/controller/direct/bigqueryconnection/connection_controller.go index fc4502d9e4..39cc632650 100644 --- a/pkg/controller/direct/bigqueryconnection/connection_controller.go +++ b/pkg/controller/direct/bigqueryconnection/connection_controller.go @@ -176,11 +176,11 @@ func (a *Adapter) Find(ctx context.Context) (bool, error) { log.V(2).Info("getting BigQueryConnectionConnection", "name", a.id.External) - id, err := a.id.ConnectionID() + _, idIsSet, err := a.id.ConnectionID() if err != nil { return false, err } - if id == "" { // resource is not yet created + if !idIsSet { // resource is not yet created return false, nil } req := &bigqueryconnectionpb.GetConnectionRequest{Name: a.id.External} @@ -220,11 +220,11 @@ func (a *Adapter) Create(ctx context.Context, createOp *directbase.CreateOperati Parent: parent, Connection: resource, } - id, err := a.id.ConnectionID() + id, isIsSet, err := a.id.ConnectionID() if err != nil { return err } - if id != "" { // this means user has specified connection ID in `spec.ResourceID` field. + if isIsSet { // during "Create", this means user has specified connection ID in `spec.ResourceID` field. req = &bigqueryconnectionpb.CreateConnectionRequest{ Parent: parent, ConnectionId: id, diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/bigqueryconnection/bigqueryconnectionconnection.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/bigqueryconnection/bigqueryconnectionconnection.md index 8c0b60a1a1..87cf75349e 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/bigqueryconnection/bigqueryconnectionconnection.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/bigqueryconnection/bigqueryconnectionconnection.md @@ -539,7 +539,7 @@ spark:

string

-

{% verbatim %}Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID is generated and stored in the `status.ExternalRef` field. For acquisition: This field must be provided to identify the connection resource to acquire.{% endverbatim %}

+

{% verbatim %}Immutable. Optional. The BigQuery Connection ID used for resource creation or acquisition. For creation: If specified, this value is used as the connection ID. If not provided, a UUID will be generated and assigned as the connection ID. For acquisition: This field must be provided to identify the connection resource to acquire.{% endverbatim %}