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

made group name configurable within resourcegroup #176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1alpha1/resource_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ type Schema struct {
// +kubebuilder:validation:Required
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="apiVersion is immutable"
APIVersion string `json:"apiVersion,omitempty"`
// The group of the resourcegroup. This is used to set the API group
// of the generated CRD. If omitted, it defaults to "kro.run".
//
// +kubebuilder:validation:Optional
// +kubebuilder:default="kro.run"
Group string `json:"group,omitempty"`
Comment on lines +63 to +68
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if it makes sense to leverage the existing apiVersion field to optionally specify the group (like how k8s can have "apps/v1" or just "v1")? This would align with k8s conventions while keep our schema simpler. Just an idea.. what do you think about this approach?

cc @rushmash91 @michaelhtm @newtondev

// The spec of the resourcegroup. Typically, this is the spec of
// the CRD that the resourcegroup is managing. This is adhering
// to the SimpleSchema spec
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/kro.run_resourcegroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ spec:
x-kubernetes-validations:
- message: apiVersion is immutable
rule: self == oldSelf
group:
default: kro.run
description: |-
The group of the resourcegroup. This is used to set the API group
of the generated CRD. If omitted, it defaults to "kro.run".
type: string
kind:
description: |-
The kind of the resourcegroup. This is used to generate
Expand Down
6 changes: 6 additions & 0 deletions helm/crds/kro.run_resourcegroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ spec:
x-kubernetes-validations:
- message: apiVersion is immutable
rule: self == oldSelf
group:
default: kro.run
description: |-
The group of the resourcegroup. This is used to set the API group
of the generated CRD. If omitted, it defaults to "kro.run".
type: string
kind:
description: |-
The kind of the resourcegroup. This is used to generate
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/resourcegroup/controller_cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ func (r *ResourceGroupReconciler) cleanupResourceGroup(ctx context.Context, rg *
log.V(1).Info("cleaning up resource group", "name", rg.Name)

// shutdown microcontroller
gvr := metadata.GetResourceGroupInstanceGVR(rg.Spec.Schema.APIVersion, rg.Spec.Schema.Kind)
gvr := metadata.GetResourceGroupInstanceGVR(rg.Spec.Schema.Group, rg.Spec.Schema.APIVersion, rg.Spec.Schema.Kind)
if err := r.shutdownResourceGroupMicroController(ctx, &gvr); err != nil {
return fmt.Errorf("failed to shutdown microcontroller: %w", err)
}

// cleanup CRD
crdName := extractCRDName(rg.Spec.Schema.Kind)
crdName := extractCRDName(rg.Spec.Schema.Group, rg.Spec.Schema.Kind)
if err := r.cleanupResourceGroupCRD(ctx, crdName); err != nil {
return fmt.Errorf("failed to cleanup CRD %s: %w", crdName, err)
}
Expand Down Expand Up @@ -75,8 +75,8 @@ func (r *ResourceGroupReconciler) cleanupResourceGroupCRD(ctx context.Context, c

// extractCRDName generates the CRD name from a given kind by converting it to plural form
// and appending the Kro domain name.
func extractCRDName(kind string) string {
func extractCRDName(group, kind string) string {
return fmt.Sprintf("%s.%s",
flect.Pluralize(strings.ToLower(kind)),
v1alpha1.KroDomainName)
group)
}
9 changes: 5 additions & 4 deletions pkg/graph/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (b *Builder) NewResourceGroup(originalCR *v1alpha1.ResourceGroup) (*Graph,
// That's because the instance status schema is inferred from the CEL expressions
// in the status field of the instance resource. Those CEL expressions refer to
// the resources defined in the resource group. Hence, we need to build the resources
// first, to be able ot generate a proper schema for the instance status.
// first, to be able to generate a proper schema for the instance status.

//

Expand All @@ -190,6 +190,7 @@ func (b *Builder) NewResourceGroup(originalCR *v1alpha1.ResourceGroup) (*Graph,
// 4. Infer the status schema based on the CEL expressions.

instance, err := b.buildInstanceResource(
rg.Spec.Schema.Group,
rg.Spec.Schema.APIVersion,
rg.Spec.Schema.Kind,
rg.Spec.Schema,
Expand Down Expand Up @@ -415,7 +416,7 @@ func (b *Builder) buildDependencyGraph(
// Since instances are defined using the "SimpleSchema" format, we use a different
// approach to build the instance resource. We need to:
func (b *Builder) buildInstanceResource(
apiVersion, kind string,
group, apiVersion, kind string,
rgDefinition *v1alpha1.Schema,
resources map[string]*Resource,
) (*Resource, error) {
Expand All @@ -428,7 +429,7 @@ func (b *Builder) buildInstanceResource(
// CRD declarations.

// The instance resource is a Kubernetes resource, so it has a GroupVersionKind.
gvk := metadata.GetResourceGroupInstanceGVK(apiVersion, kind)
gvk := metadata.GetResourceGroupInstanceGVK(group, apiVersion, kind)

// We need to unmarshal the instance schema to a map[string]interface{} to
// make it easier to work with.
Expand All @@ -451,7 +452,7 @@ func (b *Builder) buildInstanceResource(

// Synthesize the CRD for the instance resource.
overrideStatusFields := true
instanceCRD := crd.SynthesizeCRD(apiVersion, kind, *instanceSpecSchema, *instanceStatusSchema, overrideStatusFields)
instanceCRD := crd.SynthesizeCRD(group, apiVersion, kind, *instanceSpecSchema, *instanceStatusSchema, overrideStatusFields)

// Emulate the CRD
instanceSchemaExt := instanceCRD.Spec.Versions[0].Schema.OpenAPIV3Schema
Expand Down
11 changes: 5 additions & 6 deletions pkg/graph/crd/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,26 @@ import (
"fmt"
"strings"

"github.com/awslabs/kro/api/v1alpha1"
"github.com/gobuffalo/flect"
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// SynthesizeCRD generates a CustomResourceDefinition for a given API version and kind
// with the provided spec and status schemas~
func SynthesizeCRD(apiVersion, kind string, spec, status extv1.JSONSchemaProps, statusFieldsOverride bool) *extv1.CustomResourceDefinition {
return newCRD(apiVersion, kind, newCRDSchema(spec, status, statusFieldsOverride))
func SynthesizeCRD(group, apiVersion, kind string, spec, status extv1.JSONSchemaProps, statusFieldsOverride bool) *extv1.CustomResourceDefinition {
return newCRD(group, apiVersion, kind, newCRDSchema(spec, status, statusFieldsOverride))
}

func newCRD(apiVersion, kind string, schema *extv1.JSONSchemaProps) *extv1.CustomResourceDefinition {
func newCRD(group, apiVersion, kind string, schema *extv1.JSONSchemaProps) *extv1.CustomResourceDefinition {
pluralKind := flect.Pluralize(strings.ToLower(kind))
return &extv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s.%s", pluralKind, v1alpha1.KroDomainName),
Name: fmt.Sprintf("%s.%s", pluralKind, group),
OwnerReferences: nil, // Injecting owner references is the responsibility of the caller.
},
Spec: extv1.CustomResourceDefinitionSpec{
Group: v1alpha1.KroDomainName,
Group: group,
Names: extv1.CustomResourceDefinitionNames{
Kind: kind,
ListKind: kind + "List",
Expand Down
8 changes: 4 additions & 4 deletions pkg/metadata/groupversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,20 @@ func ExtractGVKFromUnstructured(unstructured map[string]interface{}) (schema.Gro
}, nil
}

func GetResourceGroupInstanceGVK(apiVersion, kind string) schema.GroupVersionKind {
func GetResourceGroupInstanceGVK(group, apiVersion, kind string) schema.GroupVersionKind {
//pluralKind := flect.Pluralize(strings.ToLower(kind))

return schema.GroupVersionKind{
Group: KroInstancesGroupSuffix,
Group: group,
Version: apiVersion,
Kind: kind,
}
}

func GetResourceGroupInstanceGVR(apiVersion, kind string) schema.GroupVersionResource {
func GetResourceGroupInstanceGVR(group, apiVersion, kind string) schema.GroupVersionResource {
pluralKind := flect.Pluralize(strings.ToLower(kind))
return schema.GroupVersionResource{
Group: fmt.Sprintf("%s.%s", pluralKind, KroInstancesGroupSuffix),
Group: fmt.Sprintf("%s.%s", pluralKind, group),
Version: apiVersion,
Resource: pluralKind,
}
Expand Down
Loading