Skip to content

Commit

Permalink
Fix chains default config
Browse files Browse the repository at this point in the history
After making Chains install through TektonConfig, for backward compatibility
to keep chains configured data preserved a mechanism was added to add
the data from chains-config configMap to TektonConfig

But if Chains is installed through TektonConfig and some default chains
config, then those were getting over written because chains-config
configMap was empty.

Hence with this patch removing this support for backward compatibilty

Fixes: #2160

Signed-off-by: PuneetPunamiya <[email protected]>
  • Loading branch information
PuneetPunamiya committed Jul 30, 2024
1 parent cab6195 commit 6893d2c
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 102 deletions.
34 changes: 0 additions & 34 deletions pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package upgrade

import (
"context"
"encoding/json"

"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
"github.com/tektoncd/operator/pkg/client/clientset/versioned"
Expand All @@ -30,39 +29,6 @@ import (
duckv1 "knative.dev/pkg/apis/duck/v1"
)

func upgradeChainProperties(ctx context.Context, logger *zap.SugaredLogger, k8sClient kubernetes.Interface, operatorClient versioned.Interface, restConfig *rest.Config) error {
// get tektonConfig CR
tc, err := operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{})
if err != nil {
logger.Errorw("error on getting TektonConfig CR", err)
return err
}

var chain v1alpha1.ChainProperties
cm, err := k8sClient.CoreV1().ConfigMaps(tc.Spec.GetTargetNamespace()).Get(ctx, "chains-config", metav1.GetOptions{})
if err != nil {
if apierrs.IsNotFound(err) {
chain = v1alpha1.ChainProperties{}
}
}
if cm != nil && len(cm.Data) > 0 {
jsonData, err := json.Marshal(cm.Data)
if err != nil {
return err
}
if err := json.Unmarshal(jsonData, &chain); err != nil {
return err
}
}

tc.Spec.Chain = v1alpha1.Chain{
ChainProperties: chain,
}

_, err = operatorClient.OperatorV1alpha1().TektonConfigs().Update(ctx, tc, metav1.UpdateOptions{})
return err
}

// previous version of tekton operator uses a condition type called "InstallSucceeded" in status
// but in the recent version we do not have that field, hence "InstallSucceeded" condition never updated.
// for some reason, if it was in failed state, tektonConfig CR always in failed state
Expand Down
66 changes: 0 additions & 66 deletions pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,77 +23,11 @@ import (
"github.com/stretchr/testify/assert"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
operatorFake "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sFake "k8s.io/client-go/kubernetes/fake"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/logging"
)

func TestUpgradeChainProperties(t *testing.T) {
ctx := context.TODO()
operatorClient := operatorFake.NewSimpleClientset()
k8sClient := k8sFake.NewSimpleClientset()
logger := logging.FromContext(ctx).Named("unit-test")

// there is no tektonConfig CR available, returns error
err := upgradeChainProperties(ctx, logger, k8sClient, operatorClient, nil)
assert.Error(t, err)

// create tekconConfig CR
tc := &v1alpha1.TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: v1alpha1.ConfigResourceName,
},
Spec: v1alpha1.TektonConfigSpec{
CommonSpec: v1alpha1.CommonSpec{
TargetNamespace: "foo",
},
Chain: v1alpha1.Chain{
ChainProperties: v1alpha1.ChainProperties{
BuilderID: "bar",
},
},
},
}
_, err = operatorClient.OperatorV1alpha1().TektonConfigs().Create(ctx, tc, metav1.CreateOptions{})
assert.NoError(t, err)

// there is no chains-config configMap, return no error
err = upgradeChainProperties(ctx, logger, k8sClient, operatorClient, nil)
assert.NoError(t, err)

// verify chains existing field, should not be empty
tc, err = operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, "", tc.Spec.Chain.ChainProperties.BuilderID)

// create a config map with values
config := &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "chains-config",
Namespace: tc.Spec.GetTargetNamespace(),
},
Data: map[string]string{
"builder.id": "123",
"transparency.enabled": "false",
"unknown_field": "hello",
},
}
_, err = k8sClient.CoreV1().ConfigMaps(tc.Spec.GetTargetNamespace()).Create(ctx, config, metav1.CreateOptions{})
assert.NoError(t, err)

// execute chains upgrade
err = upgradeChainProperties(ctx, logger, k8sClient, operatorClient, nil)
assert.NoError(t, err)

// verify chains with new configMap, map values should be updated
tc, err = operatorClient.OperatorV1alpha1().TektonConfigs().Get(ctx, v1alpha1.ConfigResourceName, metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, "123", tc.Spec.Chain.ChainProperties.BuilderID)
assert.Equal(t, v1alpha1.BoolValue("false"), tc.Spec.Chain.ChainProperties.TransparencyConfigEnabled)

}
func TestResetTektonConfigConditions(t *testing.T) {
ctx := context.TODO()
operatorClient := operatorFake.NewSimpleClientset()
Expand Down
3 changes: 1 addition & 2 deletions pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ import (
var (
// pre upgrade functions
preUpgradeFunctions = []upgradeFunc{
upgradeChainProperties, // upgrade #1: upgrade chain properties
resetTektonConfigConditions, // upgrade #2: removes conditions from TektonConfig CR, clears outdated conditions
resetTektonConfigConditions, // upgrade #1: removes conditions from TektonConfig CR, clears outdated conditions
}

// post upgrade functions
Expand Down

0 comments on commit 6893d2c

Please sign in to comment.