Skip to content

Commit

Permalink
LoggingLogMetric - kind should not be required in projectRef
Browse files Browse the repository at this point in the history
This regression was introduced because we reused the ResourceRef type.
  • Loading branch information
justinsb committed Jun 4, 2024
1 parent a7e0817 commit b045168
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 31 deletions.
3 changes: 2 additions & 1 deletion apis/monitoring/v1beta1/monitoringdashboard_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package v1beta1
import (
"reflect"

refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/k8s/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -812,7 +813,7 @@ type LogsPanel struct {
// +kcc:proto=google.monitoring.dashboard.v1.Dashboard
type MonitoringDashboardSpec struct {
/* Immutable. The Project that this resource belongs to. */
ProjectRef v1alpha1.ResourceRef `json:"projectRef"`
ProjectRef refs.ProjectRef `json:"projectRef"`

/* Immutable. Optional. The name of the resource. Used for creation and acquisition. When unset, the value of `metadata.name` is used as the default. */
// +optional
Expand Down
6 changes: 5 additions & 1 deletion apis/refs/v1beta1/projectref.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@

package v1beta1

// The Project that this resource belongs to.
type ProjectRef struct {
/* The `projectID` field of a project, when not managed by KCC. */
External string `json:"external,omitempty"`
/* The `name` field of a `Project` resource. */
Name string `json:"name,omitempty"`
/* The `namespcae` field of a `Project` resource. */
/* The `namespace` field of a `Project` resource. */
Namespace string `json:"namespace,omitempty"`
// The kind of the Project resource; optional but must be `Project` if provided.
// +optional
Kind string `json:"kind,omitempty"`
}
3 changes: 2 additions & 1 deletion apis/resources/logging/v1beta1/logmetric_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package v1beta1

import (
refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/k8s/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -176,7 +177,7 @@ type LoggingLogMetricSpec struct {
MetricDescriptor *LogmetricMetricDescriptor `json:"metricDescriptor,omitempty"`

/* Immutable. The Project that this resource belongs to. */
ProjectRef v1alpha1.ResourceRef `json:"projectRef"`
ProjectRef refs.ProjectRef `json:"projectRef"`

/* Immutable. Optional. The name of the resource. Used for creation and acquisition. When unset, the value of `metadata.name` is used as the default. */
// +optional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,15 @@ spec:
description: The `projectID` field of a project, when not managed
by KCC.
type: string
kind:
description: The kind of the Project resource; optional but must
be `Project` if provided.
type: string
name:
description: The `name` field of a `Project` resource.
type: string
namespace:
description: The `namespcae` field of a `Project` resource.
description: The `namespace` field of a `Project` resource.
type: string
type: object
resourceID:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,29 +324,28 @@ spec:
- external
required:
- name
- kind
- not:
anyOf:
- required:
- name
- required:
- namespace
- required:
- kind
required:
- external
properties:
external:
description: The external name of the referenced resource
description: The `projectID` field of a project, when not managed
by KCC.
type: string
kind:
description: Kind of the referent.
description: The kind of the Project resource; optional but must
be `Project` if provided.
type: string
name:
description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
description: The `name` field of a `Project` resource.
type: string
namespace:
description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
description: The `namespace` field of a `Project` resource.
type: string
type: object
resourceID:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4372,29 +4372,28 @@ spec:
- external
required:
- name
- kind
- not:
anyOf:
- required:
- name
- required:
- namespace
- required:
- kind
required:
- external
properties:
external:
description: The external name of the referenced resource
description: The `projectID` field of a project, when not managed
by KCC.
type: string
kind:
description: Kind of the referent.
description: The kind of the Project resource; optional but must
be `Project` if provided.
type: string
name:
description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
description: The `name` field of a `Project` resource.
type: string
namespace:
description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
description: The `namespace` field of a `Project` resource.
type: string
type: object
resourceID:
Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/direct/gkehub/featuremembership_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"

refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
krm "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/gkehub/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct/directbase"
Expand Down Expand Up @@ -77,11 +78,16 @@ func (m *gkeHubModel) AdapterForObject(ctx context.Context, reader client.Reader
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &obj); err != nil {
return nil, fmt.Errorf("error converting to %T: %w", obj, err)
}
projectRef, err := references.ResolveProject(ctx, reader, obj, &obj.Spec.ProjectRef)
projectRef := &refs.ProjectRef{
Name: obj.Spec.ProjectRef.Name,
Namespace: obj.Spec.ProjectRef.Namespace,
External: obj.Spec.ProjectRef.External,
}
project, err := references.ResolveProject(ctx, reader, obj, projectRef)
if err != nil {
return nil, err
}
projectID := projectRef.ProjectID
projectID := project.ProjectID
if projectID == "" {
return nil, fmt.Errorf("cannot resolve project")
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/controller/direct/logging/logbucketref.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"strings"

refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/k8s/v1alpha1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/logging/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct/references"
Expand Down Expand Up @@ -94,7 +95,12 @@ func LogBucketRef_ConvertToExternal(ctx context.Context, reader client.Reader, s
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(loggingLogBucket.Object, &obj); err != nil {
return fmt.Errorf("error converting LoggingLogBucket %v: %w", key, err)
}
project, err := references.ResolveProject(ctx, reader, loggingLogBucket, obj.Spec.ProjectRef)
projectRef := &refs.ProjectRef{
Name: obj.Spec.ProjectRef.Name,
Namespace: obj.Spec.ProjectRef.Namespace,
External: obj.Spec.ProjectRef.External,
}
project, err := references.ResolveProject(ctx, reader, loggingLogBucket, projectRef)
if err != nil {
return fmt.Errorf("cannot get project for referenced LoggingLogBucket %v: %w", key, err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/direct/logging/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"reflect"
"sort"

refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/resources/logging/v1beta1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/k8s/v1alpha1"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/util"
Expand Down Expand Up @@ -148,7 +149,7 @@ func convertAPItoKRM_LoggingLogMetric(projectID string, in *api.LogMetric) (*uns
}
}

lm.Spec.ProjectRef = v1alpha1.ResourceRef{
lm.Spec.ProjectRef = refs.ProjectRef{
External: projectID,
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/direct/references/projectref.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"fmt"
"strings"

"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/clients/generated/apis/k8s/v1alpha1"
refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -31,11 +31,17 @@ type Project struct {
ProjectID string
}

func ResolveProject(ctx context.Context, reader client.Reader, src client.Object, ref *v1alpha1.ResourceRef) (*Project, error) {
func ResolveProject(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ProjectRef) (*Project, error) {
if ref == nil {
return nil, nil
}

if ref.Kind != "" {
if ref.Kind != "Project" {
return nil, fmt.Errorf("kind is optional on project reference, but must be \"Project\" if provided")
}
}

if ref.External != "" {
if ref.Name != "" {
return nil, fmt.Errorf("cannot specify both name and external on project reference")
Expand Down
36 changes: 29 additions & 7 deletions scripts/add-validation-to-crds/parse-crds.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,23 @@ func processFile(ctx context.Context, p string) error {

func addRefsToCRD(crd *apiextensions.CustomResourceDefinition) error {
for _, v := range crd.Spec.Versions {
if err := addRefsToProps(v.Schema.OpenAPIV3Schema); err != nil {
if err := addRefsToProps("", v.Schema.OpenAPIV3Schema); err != nil {
return err
}
}
return nil
}

func addRefsToProps(props *apiextensions.JSONSchemaProps) error {
func addRefsToProps(fieldPath string, props *apiextensions.JSONSchemaProps) error {
// Descend into arrays
if props.Items != nil {
if props.Items.Schema != nil {
if err := addRefsToProps(props.Items.Schema); err != nil {
if err := addRefsToProps(fieldPath+"[]", props.Items.Schema); err != nil {
return err
}
}
for i := range props.Items.JSONSchemas {
if err := addRefsToProps(&props.Items.JSONSchemas[i]); err != nil {
if err := addRefsToProps(fieldPath+"[]", &props.Items.JSONSchemas[i]); err != nil {
return err
}
}
Expand All @@ -157,13 +157,13 @@ func addRefsToProps(props *apiextensions.JSONSchemaProps) error {
// Descend into objects
for k := range props.Properties {
v := props.Properties[k]
if err := addRefsToProps(&v); err != nil {
if err := addRefsToProps(fieldPath+"."+k, &v); err != nil {
return err
}
props.Properties[k] = v
}

if err := addValidationToRefs(props); err != nil {
if err := addValidationToRefs(fieldPath, props); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -206,7 +206,24 @@ oneOf:
- external
`

func addValidationToRefs(props *apiextensions.JSONSchemaProps) error {
const refRuleWithOptionalKind = `
oneOf:
- not:
required:
- external
required:
- name
- not:
anyOf:
- required:
- name
- required:
- namespace
required:
- external
`

func addValidationToRefs(fieldPath string, props *apiextensions.JSONSchemaProps) error {
// Is this a ref?
if props.Type != "object" {
return nil
Expand All @@ -223,6 +240,11 @@ func addValidationToRefs(props *apiextensions.JSONSchemaProps) error {
ruleYAML = refRuleWithKind
} else if signature == "external,kind,name,namespace" {
ruleYAML = refRuleWithKind
// kind is optional for projectRef (and maybe in future other well-known ref types)
// fieldPath is the best mechanism we have today (?)
if fieldPath == ".spec.projectRef" {
ruleYAML = refRuleWithOptionalKind
}
} else if signature == "external,name,namespace" {
ruleYAML = refRuleWithoutKind
} else {
Expand Down

0 comments on commit b045168

Please sign in to comment.