Skip to content

Commit

Permalink
Validates component CR name in webhook
Browse files Browse the repository at this point in the history
   - This patch adds a check where it validates the name
     of the component CR before applying it on the cluster

   - This patch also moves all the constants to `pkg/apis/const.go`

Fixes: #497

Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>
  • Loading branch information
PuneetPunamiya committed Nov 16, 2021
1 parent 91460be commit eafe41f
Show file tree
Hide file tree
Showing 31 changed files with 123 additions and 94 deletions.
13 changes: 12 additions & 1 deletion pkg/apis/operator/v1alpha1/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ limitations under the License.

package v1alpha1

import "fmt"
import (
"fmt"
)

var (
// RECONCILE_AGAIN_ERR
Expand Down Expand Up @@ -62,3 +64,12 @@ var (
PipelineTemplatesParam: defaultParamValue,
}
)

var (
PipelineResourceName = "pipeline"
TriggerResourceName = "trigger"
DashboardResourceName = "dashboard"
AddonResourceName = "addon"
ConfigResourceName = "config"
ResultResourceName = "result"
)
6 changes: 6 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonaddon_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"fmt"

"knative.dev/pkg/apis"
)
Expand All @@ -28,6 +29,11 @@ func (ta *TektonAddon) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if ta.GetName() != AddonResourceName {
errMsg := fmt.Sprintf("metadata.name, Only one instance of TektonAddon is allowed by name, %s", AddonResourceName)
errs = errs.Also(apis.ErrInvalidValue(ta.GetName(), errMsg))
}

if ta.Spec.TargetNamespace == "" {
errs = errs.Also(apis.ErrMissingField("spec.targetNamespace"))
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/operator/v1alpha1/tektonaddon_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func Test_ValidateTektonAddon_InvalidParam(t *testing.T) {

ta := &TektonAddon{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "addon",
Namespace: "namespace",
},
Spec: TektonAddonSpec{
Expand All @@ -73,7 +73,7 @@ func Test_ValidateTektonAddon_InvalidParamValue(t *testing.T) {

ta := &TektonAddon{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "addon",
Namespace: "namespace",
},
Spec: TektonAddonSpec{
Expand All @@ -97,7 +97,7 @@ func Test_ValidateTektonAddon_ClusterTaskIsFalseAndPipelineTemplateIsTrue(t *tes

ta := &TektonAddon{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "addon",
Namespace: "namespace",
},
Spec: TektonAddonSpec{
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonconfig_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"fmt"

"knative.dev/pkg/apis"
)
Expand All @@ -28,6 +29,11 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if tc.GetName() != ConfigResourceName {
errMsg := fmt.Sprintf("metadata.name, Only one instance of TektonConfig is allowed by name, %s", ConfigResourceName)
errs = errs.Also(apis.ErrInvalidValue(tc.GetName(), errMsg))
}

if tc.Spec.TargetNamespace == "" {
errs = errs.Also(apis.ErrMissingField("spec.targetNamespace"))
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/apis/operator/v1alpha1/tektonconfig_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func Test_ValidateTektonConfig_MissingTargetNamespace(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{},
Expand All @@ -64,7 +64,7 @@ func Test_ValidateTektonConfig_InvalidProfile(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -83,7 +83,7 @@ func Test_ValidateTektonConfig_InvalidPruningResource(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -106,7 +106,7 @@ func Test_ValidateTektonConfig_MissingKeepKeepsinceSchedule(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -128,7 +128,7 @@ func Test_ValidateTektonConfig_MissingSchedule(t *testing.T) {
keep := uint(2)
tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -151,7 +151,7 @@ func Test_ValidateTektonConfig_InvalidAddonParam(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -178,7 +178,7 @@ func Test_ValidateTektonConfig_InvalidAddonParamValue(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -205,7 +205,7 @@ func Test_ValidateTektonConfig_InvalidPipelineProperties(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand All @@ -229,7 +229,7 @@ func Test_ValidateTektonConfig_InvalidTriggerProperties(t *testing.T) {

tc := &TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "config",
Namespace: "namespace",
},
Spec: TektonConfigSpec{
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/operator/v1alpha1/tektondashboard_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"fmt"

"knative.dev/pkg/apis"
)
Expand All @@ -28,6 +29,11 @@ func (td *TektonDashboard) Validate(ctx context.Context) (errs *apis.FieldError)
return nil
}

if td.GetName() != DashboardResourceName {
errMsg := fmt.Sprintf("metadata.name, Only one instance of TektonDashboard is allowed by name, %s", DashboardResourceName)
errs = errs.Also(apis.ErrInvalidValue(td.GetName(), errMsg))
}

if td.Spec.TargetNamespace == "" {
errs = errs.Also(apis.ErrMissingField("spec.targetNamespace"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func Test_ValidateTektonDashboard_MissingTargetNamespace(t *testing.T) {

td := &TektonDashboard{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "dashboard",
Namespace: "namespace",
},
Spec: TektonDashboardSpec{},
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonpipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"fmt"

"knative.dev/pkg/apis"
)
Expand All @@ -28,6 +29,11 @@ func (tp *TektonPipeline) Validate(ctx context.Context) (errs *apis.FieldError)
return nil
}

if tp.GetName() != PipelineResourceName {
errMsg := fmt.Sprintf("metadata.name, Only one instance of TektonPipeline is allowed by name, %s", PipelineResourceName)
errs = errs.Also(apis.ErrInvalidValue(tp.GetName(), errMsg))
}

if tp.Spec.TargetNamespace == "" {
errs = errs.Also(apis.ErrMissingField("spec.targetNamespace"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func Test_ValidateTektonPipeline_MissingTargetNamespace(t *testing.T) {

tp := &TektonPipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "pipeline",
Namespace: "namespace",
},
Spec: TektonPipelineSpec{},
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/operator/v1alpha1/tektontrigger_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
"fmt"

"knative.dev/pkg/apis"
)
Expand All @@ -28,6 +29,11 @@ func (tr *TektonTrigger) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if tr.GetName() != TriggerResourceName {
errMsg := fmt.Sprintf("metadata.name, Only one instance of TektonTrigger is allowed by name, %s", TriggerResourceName)
errs = errs.Also(apis.ErrInvalidValue(tr.GetName(), errMsg))
}

if tr.Spec.TargetNamespace == "" {
errs = errs.Also(apis.ErrMissingField("spec.targetNamespace"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func Test_ValidateTektonTrigger_MissingTargetNamespace(t *testing.T) {

tr := &TektonTrigger{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Name: "trigger",
Namespace: "namespace",
},
Spec: TektonTriggerSpec{},
Expand Down
19 changes: 5 additions & 14 deletions pkg/reconciler/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,10 @@ import (
)

var (
Interval = 10 * time.Second
Timeout = 1 * time.Minute
// DefaultSA is the default service account
DefaultSA = "pipeline"
PipelineResourceName = "pipeline"
TriggerResourceName = "trigger"
DashboardResourceName = "dashboard"
AddonResourceName = "addon"
ConfigResourceName = "config"
ResultResourceName = "result"
ProfileLite = "lite"
ProfileBasic = "basic"
ProfileAll = "all"
Interval = 10 * time.Second
Timeout = 1 * time.Minute
DefaultSA = "pipeline"
)

const (
Expand Down Expand Up @@ -68,7 +59,7 @@ func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPi
}

func getPipelineRes(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPipeline, error) {
res, err := informer.Lister().Get(PipelineResourceName)
res, err := informer.Lister().Get(v1alpha1.PipelineResourceName)
return res, err
}

Expand All @@ -90,6 +81,6 @@ func TriggerReady(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrig
}

func getTriggerRes(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrigger, error) {
res, err := informer.Lister().Get(TriggerResourceName)
res, err := informer.Lister().Get(v1alpha1.TriggerResourceName)
return res, err
}
10 changes: 5 additions & 5 deletions pkg/reconciler/kubernetes/tektonconfig/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,20 @@ func (oe kubernetesExtension) PreReconcile(context.Context, v1alpha1.TektonCompo
func (oe kubernetesExtension) PostReconcile(ctx context.Context, comp v1alpha1.TektonComponent) error {
configInstance := comp.(*v1alpha1.TektonConfig)

if configInstance.Spec.Profile == common.ProfileAll {
if configInstance.Spec.Profile == v1alpha1.ProfileAll {
return extension.CreateDashboardCR(comp, oe.operatorClientSet.OperatorV1alpha1())
}

if configInstance.Spec.Profile == common.ProfileLite || configInstance.Spec.Profile == common.ProfileBasic {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), common.DashboardResourceName)
if configInstance.Spec.Profile == v1alpha1.ProfileLite || configInstance.Spec.Profile == v1alpha1.ProfileBasic {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), v1alpha1.DashboardResourceName)
}

return nil
}
func (oe kubernetesExtension) Finalize(ctx context.Context, comp v1alpha1.TektonComponent) error {
configInstance := comp.(*v1alpha1.TektonConfig)
if configInstance.Spec.Profile == common.ProfileAll {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), common.DashboardResourceName)
if configInstance.Spec.Profile == v1alpha1.ProfileAll {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), v1alpha1.DashboardResourceName)
}
return nil
}
8 changes: 4 additions & 4 deletions pkg/reconciler/kubernetes/tektonconfig/extension/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func CreateDashboardCR(instance v1alpha1.TektonComponent, client operatorv1alpha
if _, err := ensureTektonDashboardExists(client.TektonDashboards(), configInstance); err != nil {
return errors.New(err.Error())
}
if _, err := waitForTektonDashboardState(client.TektonDashboards(), common.DashboardResourceName,
if _, err := waitForTektonDashboardState(client.TektonDashboards(), v1alpha1.DashboardResourceName,
isTektonDashboardReady); err != nil {
log.Println("TektonDashboard is not in ready state: ", err)
return err
Expand All @@ -47,7 +47,7 @@ func CreateDashboardCR(instance v1alpha1.TektonComponent, client operatorv1alpha
}

func ensureTektonDashboardExists(clients op.TektonDashboardInterface, config *v1alpha1.TektonConfig) (*v1alpha1.TektonDashboard, error) {
tdCR, err := GetDashboard(clients, common.DashboardResourceName)
tdCR, err := GetDashboard(clients, v1alpha1.DashboardResourceName)
if err == nil {
// if the dashboard spec is changed then update the instance
updated := false
Expand Down Expand Up @@ -83,7 +83,7 @@ func ensureTektonDashboardExists(clients op.TektonDashboardInterface, config *v1
if apierrs.IsNotFound(err) {
tdCR = &v1alpha1.TektonDashboard{
ObjectMeta: metav1.ObjectMeta{
Name: common.DashboardResourceName,
Name: v1alpha1.DashboardResourceName,
},
Spec: v1alpha1.TektonDashboardSpec{
CommonSpec: v1alpha1.CommonSpec{
Expand Down Expand Up @@ -128,7 +128,7 @@ func isTektonDashboardReady(s *v1alpha1.TektonDashboard, err error) (bool, error

// TektonDashboardCRDelete deletes tha TektonDashboard to see if all resources will be deleted
func TektonDashboardCRDelete(clients op.TektonDashboardInterface, name string) error {
if _, err := GetDashboard(clients, common.DashboardResourceName); err != nil {
if _, err := GetDashboard(clients, v1alpha1.DashboardResourceName); err != nil {
if apierrs.IsNotFound(err) {
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ package extension
import (
"testing"

"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
"github.com/tektoncd/operator/pkg/client/injection/client/fake"
"github.com/tektoncd/operator/pkg/reconciler/common"
util "github.com/tektoncd/operator/pkg/reconciler/common/testing"
"github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig/pipeline"
ts "knative.dev/pkg/reconciler/testing"
Expand All @@ -32,13 +32,13 @@ func TestTektonDashboardCreateAndDeleteCR(t *testing.T) {
tConfig := pipeline.GetTektonConfig()
err := CreateDashboardCR(tConfig, c.OperatorV1alpha1())
util.AssertNotEqual(t, err, nil)
err = TektonDashboardCRDelete(c.OperatorV1alpha1().TektonDashboards(), common.DashboardResourceName)
err = TektonDashboardCRDelete(c.OperatorV1alpha1().TektonDashboards(), v1alpha1.DashboardResourceName)
util.AssertEqual(t, err, nil)
}

func TestTektonDashboardCRDelete(t *testing.T) {
ctx, _, _ := ts.SetupFakeContextWithCancel(t)
c := fake.Get(ctx)
err := TektonDashboardCRDelete(c.OperatorV1alpha1().TektonDashboards(), common.DashboardResourceName)
err := TektonDashboardCRDelete(c.OperatorV1alpha1().TektonDashboards(), v1alpha1.DashboardResourceName)
util.AssertEqual(t, err, nil)
}
Loading

0 comments on commit eafe41f

Please sign in to comment.