Skip to content

Commit

Permalink
fix: reject pool creation if pool with spec exists
Browse files Browse the repository at this point in the history
in garm, a pool is uniq on imageName, flavor, provider and scope.
This fixes the current implementation where we compared the image.tag
name with the name of the image referenc itself.

Signed-off-by: Mario Constanti <[email protected]>
  • Loading branch information
bavarianbidi committed Sep 18, 2023
1 parent a35bf35 commit fc58124
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ $(GOLANGCI_LINT): $(LOCALBIN)
.PHONY: mockgen
mockgen: $(MOCKGEN) ## Download mockgen locally if necessary. If wrong version is installed, it will be overwritten.
$(MOCKGEN): $(LOCALBIN)
test -s $(LOCALBIN)/mockgen && $(LOCALBIN)/mockget --version | grep -q $(MOCKGEN_VERSION) || \
test -s $(LOCALBIN)/mockgen && $(LOCALBIN)/mockgen --version | grep -q $(MOCKGEN_VERSION) || \
GOBIN=$(LOCALBIN) go install go.uber.org/mock/mockgen@$(MOCKGEN_VERSION)

.PHONY: goreleaser
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<!-- SPDX-License-Identifier: MIT -->

[![Go Report Card](https://goreportcard.com/badge/github.com/mercedes-benz/garm-operator)](https://goreportcard.com/report/github.com/mercedes-benz/garm-operator) ![GitHub release (latest SemVer)](https://img.shields.io/github/v/release/mercedes-benz/garm-operator?sort=semver)

# garm-operator

<!-- toc -->
Expand Down
148 changes: 148 additions & 0 deletions api/v1alpha1/pool_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// SPDX-License-Identifier: MIT

package v1alpha1

import (
"testing"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
)

func TestPoolList_FilterByFields(t *testing.T) {
type fields struct {
TypeMeta metav1.TypeMeta
ListMeta metav1.ListMeta
Items []Pool
}
type args struct {
predicates []Predicate
}
tests := []struct {
name string
fields fields
args args
length int
}{
{
name: "pool with spec already exist",
fields: fields{
TypeMeta: metav1.TypeMeta{},
ListMeta: metav1.ListMeta{},
Items: []Pool{
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "ubuntu-2004-large",
Namespace: "test",
},
Spec: PoolSpec{
ImageName: "ubuntu-2004",
Flavor: "large",
ProviderName: "openstack",
GitHubScopeRef: corev1.TypedLocalObjectReference{
Name: "test",
Kind: "Enterprise",
APIGroup: ptr.To[string]("github.com"),
},
},
},
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "ubuntu-2204-large",
Namespace: "test",
},
Spec: PoolSpec{
ImageName: "ubuntu-2204",
Flavor: "large",
ProviderName: "openstack",
GitHubScopeRef: corev1.TypedLocalObjectReference{
Name: "test",
Kind: "Enterprise",
APIGroup: ptr.To[string]("github.com"),
},
},
},
},
},
args: args{
predicates: []Predicate{
MatchesImage("ubuntu-2204"),
MatchesFlavour("large"),
MatchesProvider("openstack"),
MatchesGitHubScope("test", "Enterprise"),
},
},
length: 1,
},
{
name: "pool with spec does not exist",
fields: fields{
TypeMeta: metav1.TypeMeta{},
ListMeta: metav1.ListMeta{},
Items: []Pool{
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "ubuntu-2004-large",
Namespace: "test",
},
Spec: PoolSpec{
ImageName: "ubuntu-2004",
Flavor: "large",
ProviderName: "openstack",
GitHubScopeRef: corev1.TypedLocalObjectReference{
Name: "test",
Kind: "Enterprise",
APIGroup: ptr.To[string]("github.com"),
},
},
},
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "ubuntu-2204-large",
Namespace: "test",
},
Spec: PoolSpec{
ImageName: "ubuntu-2204",
Flavor: "large",
ProviderName: "openstack",
GitHubScopeRef: corev1.TypedLocalObjectReference{
Name: "test",
Kind: "Enterprise",
APIGroup: ptr.To[string]("github.com"),
},
},
},
},
},
args: args{
predicates: []Predicate{
MatchesImage("ubuntu-2404"),
MatchesFlavour("large"),
MatchesProvider("openstack"),
MatchesGitHubScope("test", "Enterprise"),
},
},
length: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := &PoolList{
TypeMeta: tt.fields.TypeMeta,
ListMeta: tt.fields.ListMeta,
Items: tt.fields.Items,
}

p.FilterByFields(tt.args.predicates...)

if len(p.Items) != tt.length {
t.Errorf("FilterByFields() = %v, want %v", len(p.Items), tt.length)
}
})
}
}
23 changes: 11 additions & 12 deletions api/v1alpha1/pool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,12 @@ var _ webhook.Validator = &Pool{}
func (r *Pool) ValidateCreate() (admission.Warnings, error) {
poollog.Info("validate create", "name", r.Name)
ctx := context.TODO()
pool := r

image, err := validateImageName(ctx, pool)
err := validateImageName(ctx, r)
if err != nil {
return nil, apierrors.NewInvalid(
schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"},
pool.Name,
r.Name,
field.ErrorList{err},
)
}
Expand All @@ -59,7 +58,7 @@ func (r *Pool) ValidateCreate() (admission.Warnings, error) {
}

listOpts := &client.ListOptions{
Namespace: pool.Namespace,
Namespace: r.Namespace,
}

poolList := &PoolList{}
Expand All @@ -69,10 +68,10 @@ func (r *Pool) ValidateCreate() (admission.Warnings, error) {
}

poolList.FilterByFields(
MatchesFlavour(pool.Spec.Flavor),
MatchesImage(image.Spec.Tag),
MatchesProvider(pool.Spec.ProviderName),
MatchesGitHubScope(pool.Spec.GitHubScopeRef.Name, pool.Spec.GitHubScopeRef.Kind),
MatchesFlavour(r.Spec.Flavor),
MatchesImage(r.Spec.ImageName),
MatchesProvider(r.Spec.ProviderName),
MatchesGitHubScope(r.Spec.GitHubScopeRef.Name, r.Spec.GitHubScopeRef.Kind),
)

if len(poolList.Items) > 0 {
Expand All @@ -93,7 +92,7 @@ func (r *Pool) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
return nil, apierrors.NewBadRequest("failed to convert runtime.Object to Pool CRD")
}

_, err := validateImageName(context.Background(), r)
err := validateImageName(context.Background(), r)
if err != nil {
return nil, apierrors.NewInvalid(
schema.GroupKind{Group: GroupVersion.Group, Kind: "Pool"},
Expand Down Expand Up @@ -128,17 +127,17 @@ func (r *Pool) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
return nil, nil
}

func validateImageName(ctx context.Context, r *Pool) (*Image, *field.Error) {
func validateImageName(ctx context.Context, r *Pool) *field.Error {
image := Image{}
err := c.Get(ctx, client.ObjectKey{Name: r.Spec.ImageName, Namespace: r.Namespace}, &image)
if err != nil {
return nil, field.Invalid(
return field.Invalid(
field.NewPath("spec").Child("imageName"),
r.Spec.ImageName,
err.Error(),
)
}
return &image, nil
return nil
}

func (r *Pool) validateProviderName(old *Pool) *field.Error {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
k8s.io/apimachinery v0.27.2
k8s.io/client-go v0.27.2
k8s.io/klog/v2 v2.90.1
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
sigs.k8s.io/controller-runtime v0.15.0
)

Expand Down Expand Up @@ -82,7 +83,6 @@ require (
k8s.io/apiextensions-apiserver v0.27.2 // indirect
k8s.io/component-base v0.27.2 // indirect
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
Expand Down

0 comments on commit fc58124

Please sign in to comment.