Skip to content

Commit

Permalink
Add ResourceReference Validator (#1513)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthchr authored May 28, 2021
1 parent b895cb4 commit 662f514
Show file tree
Hide file tree
Showing 25 changed files with 749 additions and 69 deletions.
31 changes: 31 additions & 0 deletions hack/generated/pkg/genruntime/resource_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ package genruntime
import (
"fmt"
"reflect"

"github.com/pkg/errors"
kerrors "k8s.io/apimachinery/pkg/util/errors"
)

// KnownResourceReference is a resource reference to a known type.
Expand Down Expand Up @@ -67,6 +70,24 @@ func (ref ResourceReference) String() string {
return fmt.Sprintf("Group: %q, Kind: %q, Namespace: %q, Name: %q, ARMID: %q", ref.Group, ref.Kind, ref.Namespace, ref.Name, ref.ARMID)
}

// TODO: We wouldn't need this if controller-gen supported DUs or OneOf better, see: https://github.com/kubernetes-sigs/controller-tools/issues/461
// Validate validates the ResourceReference to ensure that it is structurally valid.
func (ref ResourceReference) Validate() error {
if ref.ARMID == "" && ref.Namespace == "" && ref.Name == "" && ref.Group == "" && ref.Kind == "" {
return errors.Errorf("at least one of ['ARMID'] or ['Group', 'Kind', 'Namespace', 'Name'] must be set for ResourceReference")
}

if ref.ARMID != "" && !ref.IsDirectARMReference() {
return errors.Errorf("the 'ARMID' field is mutually exclusive with 'Group', 'Kind', 'Namespace', and 'Name' for ResourceReference: %s", ref.String())
}

if ref.ARMID == "" && !ref.IsKubernetesReference() {
return errors.Errorf("when referencing a Kubernetes resource, 'Group', 'Kind', 'Namespace', and 'Name' must all be specified for ResourceReference: %s", ref.String())
}

return nil
}

// LookupOwnerGroupKind looks up an owners group and kind annotations using reflection.
// This is primarily used to convert from a KnownResourceReference to the more general
// ResourceReference
Expand Down Expand Up @@ -95,3 +116,13 @@ func (ref KnownResourceReference) Copy() KnownResourceReference {
func (ref ResourceReference) Copy() ResourceReference {
return ref
}

// ValidateResourceReferences calls Validate on each ResourceReference
func ValidateResourceReferences(refs map[ResourceReference]struct{}) error {
var errs []error
for ref := range refs {
errs = append(errs, ref.Validate())
}

return kerrors.NewAggregate(errs)
}
117 changes: 117 additions & 0 deletions hack/generated/pkg/genruntime/resource_reference_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
Copyright (c) Microsoft Corporation.
Licensed under the MIT license.
*/

package genruntime_test

import (
"testing"

. "github.com/onsi/gomega"

"github.com/Azure/azure-service-operator/hack/generated/pkg/genruntime"
)

var validARMIDRef = genruntime.ResourceReference{ARMID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/microsoft.compute/VirtualMachine/myvm"}
var validKubRef = genruntime.ResourceReference{Group: "microsoft.resources.infra.azure.com", Kind: "ResourceGroup", Namespace: "default", Name: "myrg"}
var invalidRefBothSpecified = genruntime.ResourceReference{Group: "microsoft.resources.infra.azure.com", Kind: "ResourceGroup", Namespace: "default", Name: "myrg", ARMID: "oops"}
var invalidRefNeitherSpecified = genruntime.ResourceReference{}
var invalidRefIncompleteKubReference = genruntime.ResourceReference{Group: "microsoft.resources.infra.azure.com", Namespace: "default", Name: "myrg"}

func Test_ResourceReference_Validate(t *testing.T) {
tests := []struct {
name string
ref genruntime.ResourceReference
errSubstring string
}{
{
name: "valid ARM reference is valid",
ref: validARMIDRef,
errSubstring: "",
},
{
name: "valid Kubernetes reference is valid",
ref: validKubRef,
errSubstring: "",
},
{
name: "both ARM and Kubernetes fields filled out, reference is invalid",
ref: invalidRefBothSpecified,
errSubstring: "'ARMID' field is mutually exclusive with",
},
{
name: "nothing filled out, reference is invalid",
ref: invalidRefNeitherSpecified,
errSubstring: "at least one of ['ARMID'] or ['Group', 'Kind', 'Namespace', 'Name'] must be set for ResourceReference",
},
{
name: "incomplete Kubernetes reference is invalid",
ref: invalidRefIncompleteKubReference,
errSubstring: "'Group', 'Kind', 'Namespace', and 'Name' must all be specified",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

err := tt.ref.Validate()
if tt.errSubstring != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tt.errSubstring))
} else {
g.Expect(err).ToNot(HaveOccurred())
}
})
}
}

func Test_ResourceReference_IsARMOrKubernetes(t *testing.T) {
tests := []struct {
name string
ref genruntime.ResourceReference
isARM bool
isKubernetes bool
}{
{
name: "valid ARM reference is ARM",
ref: validARMIDRef,
isARM: true,
isKubernetes: false,
},
{
name: "valid Kubernetes reference is Kubernetes",
ref: validKubRef,
isARM: false,
isKubernetes: true,
},
{
name: "both ARM and Kubernetes fields filled out, reference is neither",
ref: invalidRefBothSpecified,
isARM: false,
isKubernetes: false,
},
{
name: "nothing filled out, reference is neither",
ref: invalidRefNeitherSpecified,
isARM: false,
isKubernetes: false,
},
{
name: "incomplete Kubernetes reference is neither",
ref: invalidRefIncompleteKubReference,
isARM: false,
isKubernetes: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

g.Expect(tt.ref.IsDirectARMReference()).To(Equal(tt.isARM))
g.Expect(tt.ref.IsKubernetesReference()).To(Equal(tt.isKubernetes))
})
}
}
21 changes: 10 additions & 11 deletions hack/generated/pkg/reflecthelpers/reflect_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT license.
*/

package reflecthelpers
package reflecthelpers_test

import (
"context"
Expand All @@ -15,14 +15,13 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

//nolint:staticcheck // ignoring deprecation (SA1019) to unblock CI builds
"sigs.k8s.io/controller-runtime/pkg/client/fake"

resources "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.resources/v1alpha1api20200601"
"github.com/Azure/azure-service-operator/hack/generated/pkg/genruntime"
"github.com/Azure/azure-service-operator/hack/generated/pkg/reflecthelpers"
"github.com/Azure/azure-service-operator/hack/generated/pkg/util/kubeclient"

// TODO: Do we want to use a sample object rather than a code generated one?
batch "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.batch/v1alpha1api20170901"

Expand Down Expand Up @@ -141,7 +140,7 @@ func Test_ConvertResourceToDeployableResource(t *testing.T) {
account := createDummyResource()
g.Expect(test.client.Create(ctx, account)).To(Succeed())

resource, err := ConvertResourceToDeployableResource(ctx, test.resolver, account)
resource, err := reflecthelpers.ConvertResourceToDeployableResource(ctx, test.resolver, account)
g.Expect(err).ToNot(HaveOccurred())

rgResource, ok := resource.(*genruntime.ResourceGroupResource)
Expand All @@ -168,7 +167,7 @@ func Test_FindReferences(t *testing.T) {
}
g.Expect(test.client.Create(ctx, account)).To(Succeed())

refs, err := FindResourceReferences(&account.Spec)
refs, err := reflecthelpers.FindResourceReferences(&account.Spec)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(refs).To(HaveLen(1))
g.Expect(refs).To(HaveKey(ref))
Expand All @@ -179,7 +178,7 @@ func Test_NewStatus(t *testing.T) {

account := createDummyResource()

status, err := NewEmptyStatus(account)
status, err := reflecthelpers.NewEmptyStatus(account)
g.Expect(err).To(BeNil())
g.Expect(status).To(BeAssignableToTypeOf(&batch.BatchAccount_Status{}))
}
Expand All @@ -189,7 +188,7 @@ func Test_EmptyArmResourceStatus(t *testing.T) {

account := createDummyResource()

status, err := NewEmptyArmResourceStatus(account)
status, err := reflecthelpers.NewEmptyArmResourceStatus(account)
g.Expect(err).To(BeNil())
g.Expect(status).To(BeAssignableToTypeOf(&batch.BatchAccount_StatusARM{}))
}
Expand All @@ -198,7 +197,7 @@ func Test_HasStatus(t *testing.T) {
g := NewGomegaWithT(t)

account := createDummyResource()
result, err := HasStatus(account)
result, err := reflecthelpers.HasStatus(account)
g.Expect(err).To(BeNil())
g.Expect(result).To(BeFalse())
}
Expand All @@ -207,15 +206,15 @@ func Test_NewPtrFromStruct_ReturnsPtr(t *testing.T) {
g := NewGomegaWithT(t)

v := DummyStruct{}
ptr := NewPtrFromValue(v)
ptr := reflecthelpers.NewPtrFromValue(v)
g.Expect(ptr).To(BeAssignableToTypeOf(&DummyStruct{}))
}

func Test_NewPtrFromPrimitive_ReturnsPtr(t *testing.T) {
g := NewGomegaWithT(t)

v := 5
ptr := NewPtrFromValue(v)
ptr := reflecthelpers.NewPtrFromValue(v)

expectedValue := &v

Expand All @@ -226,6 +225,6 @@ func Test_NewPtrFromPtr_ReturnsPtrPtr(t *testing.T) {
g := NewGomegaWithT(t)

ptr := &DummyStruct{}
ptrPtr := NewPtrFromValue(ptr)
ptrPtr := reflecthelpers.NewPtrFromValue(ptr)
g.Expect(ptrPtr).To(BeAssignableToTypeOf(&ptr))
}
85 changes: 85 additions & 0 deletions hack/generator/pkg/astmodel/kubernetes_admissions_validations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
*/

package astmodel

import (
"go/token"

"github.com/dave/dst"

"github.com/Azure/azure-service-operator/hack/generator/pkg/astbuilder"
)

func NewValidateResourceReferencesFunction(resource *ResourceType, idFactory IdentifierFactory) *resourceFunction {
return &resourceFunction{
name: "validateResourceReferences",
resource: resource,
idFactory: idFactory,
asFunc: validateResourceReferences,
requiredPackages: NewPackageReferenceSet(GenRuntimeReference, ReflectHelpersReference),
}
}

func validateResourceReferences(k *resourceFunction, codeGenerationContext *CodeGenerationContext, receiver TypeName, methodName string) *dst.FuncDecl {
receiverIdent := k.idFactory.CreateIdentifier(receiver.Name(), NotExported)
receiverType := receiver.AsType(codeGenerationContext)

fn := &astbuilder.FuncDetails{
Name: methodName,
ReceiverIdent: receiverIdent,
ReceiverType: &dst.StarExpr{
X: receiverType,
},
Returns: []*dst.Field{
{
Type: dst.NewIdent("error"),
},
},
Body: validateResourceReferencesBody(codeGenerationContext, receiverIdent),
}

fn.AddComments("validates all resource references")
return fn.DefineFunc()
}

// validateResourceReferencesBody helps generate the body of the validateResourceReferences function:
// refs, err := reflecthelpers.FindResourceReferences(&<resource>.Spec)
// if err != nil {
// return err
// }
// return genruntime.ValidateResourceReferences(refs)
func validateResourceReferencesBody(codeGenerationContext *CodeGenerationContext, receiverIdent string) []dst.Stmt {
reflectHelpers, err := codeGenerationContext.GetImportedPackageName(ReflectHelpersReference)
if err != nil {
panic(err)
}

genRuntime, err := codeGenerationContext.GetImportedPackageName(GenRuntimeReference)
if err != nil {
panic(err)
}
var body []dst.Stmt

body = append(
body,
astbuilder.SimpleAssignmentWithErr(
dst.NewIdent("refs"),
token.DEFINE,
astbuilder.CallQualifiedFunc(
reflectHelpers,
"FindResourceReferences",
astbuilder.AddrOf(astbuilder.Selector(dst.NewIdent(receiverIdent), "Spec")))))
body = append(body, astbuilder.CheckErrorAndReturn())
body = append(
body,
astbuilder.Returns(
astbuilder.CallQualifiedFunc(
genRuntime,
"ValidateResourceReferences",
dst.NewIdent("refs"))))

return body
}
Loading

0 comments on commit 662f514

Please sign in to comment.