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 11, 2021
1 parent ed3d6b1 commit df78e1f
Show file tree
Hide file tree
Showing 36 changed files with 170 additions and 139 deletions.
26 changes: 25 additions & 1 deletion pkg/apis/operator/v1alpha1/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package v1alpha1

import "fmt"
import (
"fmt"
"time"
)

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

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

const (
PipelineNotReady = "tekton-pipelines not ready"
PipelineNotFound = "tekton-pipelines not installed"
TriggerNotReady = "tekton-triggers not ready"
TriggerNotFound = "tekton-triggers not installed"
NamespaceIgnorePattern = "^(openshift|kube)-"
)
4 changes: 4 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonaddon_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ func (ta *TektonAddon) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if ta.GetName() != AddonResourceName {
errs = errs.Also(apis.ErrInvalidValue(ta.GetName(), "metadata.name, Only one instance of TektonAddon is allowed by name `addon`"))
}

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
4 changes: 4 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonconfig_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if tc.GetName() != ConfigResourceName {
errs = errs.Also(apis.ErrInvalidValue(tc.GetName(), "metadata.name, Only one instance of TektonConfig is allowed by name `config`"))
}

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
4 changes: 4 additions & 0 deletions pkg/apis/operator/v1alpha1/tektondashboard_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ func (td *TektonDashboard) Validate(ctx context.Context) (errs *apis.FieldError)
return nil
}

if td.GetName() != DashboardResourceName {
errs = errs.Also(apis.ErrInvalidValue(td.GetName(), "metadata.name, Only one instance of TektonDashboard is allowed by name `dashboard`"))
}

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
4 changes: 4 additions & 0 deletions pkg/apis/operator/v1alpha1/tektonpipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ func (tp *TektonPipeline) Validate(ctx context.Context) (errs *apis.FieldError)
return nil
}

if tp.GetName() != PipelineResourceName {
errs = errs.Also(apis.ErrInvalidValue(tp.GetName(), "metadata.name, Only one instance of TektonPipeline is allowed by name `pipeline`"))
}

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
4 changes: 4 additions & 0 deletions pkg/apis/operator/v1alpha1/tektontrigger_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ func (tr *TektonTrigger) Validate(ctx context.Context) (errs *apis.FieldError) {
return nil
}

if tr.GetName() != TriggerResourceName {
errs = errs.Also(apis.ErrInvalidValue(tr.GetName(), "metadata.name, Only one instance of TektonTrigger is allowed by name `trigger`"))
}

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
38 changes: 7 additions & 31 deletions pkg/reconciler/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,78 +18,54 @@ package common

import (
"fmt"
"time"

"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
res "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
informer "github.com/tektoncd/operator/pkg/client/informers/externalversions/operator/v1alpha1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
)

var (
// 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
)

const (
PipelineNotReady = "tekton-pipelines not ready"
PipelineNotFound = "tekton-pipelines not installed"
TriggerNotReady = "tekton-triggers not ready"
TriggerNotFound = "tekton-triggers not installed"
NamespaceIgnorePattern = "^(openshift|kube)-"
)

func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPipeline, error) {
ppln, err := getPipelineRes(informer)
if err != nil {
if apierrors.IsNotFound(err) {
return nil, fmt.Errorf(PipelineNotFound)
return nil, fmt.Errorf(res.PipelineNotFound)
}
return nil, err
}

if len(ppln.Status.Conditions) != 0 {
if ppln.Status.Conditions[0].Status != corev1.ConditionTrue {
return nil, fmt.Errorf(PipelineNotReady)
return nil, fmt.Errorf(res.PipelineNotReady)
}
}
return ppln, nil
}

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

func TriggerReady(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrigger, error) {
trigger, err := getTriggerRes(informer)
if err != nil {
if apierrors.IsNotFound(err) {
return nil, fmt.Errorf(TriggerNotFound)
return nil, fmt.Errorf(res.TriggerNotFound)
}
return nil, err
}

if len(trigger.Status.Conditions) != 0 {
if trigger.Status.Conditions[0].Status != corev1.ConditionTrue {
return nil, fmt.Errorf(TriggerNotReady)
return nil, fmt.Errorf(res.TriggerNotReady)
}
}
return trigger, nil
}

func getTriggerRes(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrigger, error) {
res, err := informer.Lister().Get(TriggerResourceName)
res, err := informer.Lister().Get(res.TriggerResourceName)
return res, err
}
3 changes: 2 additions & 1 deletion pkg/reconciler/common/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"

"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
res "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -146,7 +147,7 @@ func prunableNamespaces(ctx context.Context, k kubernetes.Interface, defaultPrun
var prunableNs pruningNs
commonSchedule := make(map[string]*pruneConfigPerNS)
uniqueSchedule := make(map[string]*pruneConfigPerNS)
re := regexp.MustCompile(NamespaceIgnorePattern)
re := regexp.MustCompile(res.NamespaceIgnorePattern)
for _, ns := range nsList.Items {
if ignore := re.MatchString(ns.GetName()); ignore {
continue
Expand Down
11 changes: 6 additions & 5 deletions pkg/reconciler/kubernetes/tektonconfig/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

mf "github.com/manifestival/manifestival"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
res "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
"github.com/tektoncd/operator/pkg/client/clientset/versioned"
operatorclient "github.com/tektoncd/operator/pkg/client/injection/client"
"github.com/tektoncd/operator/pkg/reconciler/common"
Expand All @@ -46,20 +47,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 == res.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 == res.ProfileLite || configInstance.Spec.Profile == res.ProfileBasic {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), res.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 == res.ProfileAll {
return extension.TektonDashboardCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonDashboards(), res.DashboardResourceName)
}
return nil
}
Loading

0 comments on commit df78e1f

Please sign in to comment.