Skip to content

Commit

Permalink
Update triggers core interceptor to create secret when necessary
Browse files Browse the repository at this point in the history
The core interceptor will create secrets in only 2 cases:
1. when there is no secret present in the cluster.
2. when certificates expire within the existing secret.

If secret exist and have valid certificates no need to recreate secret everytime

Signed-off-by: Savita Ashture <sashture@redhat.com>
  • Loading branch information
savitaashture authored and tekton-robot committed Nov 15, 2023
1 parent d07613e commit 4543a07
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 11 deletions.
23 changes: 18 additions & 5 deletions pkg/interceptors/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"strings"
"time"

"go.uber.org/zap"

"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
triggersv1alpha1 "github.com/tektoncd/triggers/pkg/client/clientset/versioned/typed/triggers/v1alpha1"
Expand All @@ -25,6 +23,7 @@ import (
"github.com/tektoncd/triggers/pkg/interceptors/github"
"github.com/tektoncd/triggers/pkg/interceptors/gitlab"
"github.com/tektoncd/triggers/pkg/interceptors/slack"
"go.uber.org/zap"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
Expand Down Expand Up @@ -160,7 +159,7 @@ func (is *Server) ExecuteInterceptor(r *http.Request) ([]byte, error) {
}

func CreateAndValidateCerts(ctx context.Context, coreV1Interface corev1.CoreV1Interface, logger *zap.SugaredLogger, service *Server, tc triggersv1alpha1.TriggersV1alpha1Interface) {
serverCert, caCert, err := createCerts(ctx, coreV1Interface, time.Now().Add(Decade), logger)
serverCert, caCert, err := createCerts(ctx, coreV1Interface, time.Now().Add(Decade), logger, false)
if err != nil {
return
}
Expand All @@ -173,7 +172,8 @@ func CreateAndValidateCerts(ctx context.Context, coreV1Interface corev1.CoreV1In
service.checkCertValidity(ctx, serverCert, caCert, coreV1Interface, logger, tc, time.Minute)
}

func createCerts(ctx context.Context, coreV1Interface corev1.CoreV1Interface, noAfter time.Time, logger *zap.SugaredLogger) ([]byte, []byte, error) {
func createCerts(ctx context.Context, coreV1Interface corev1.CoreV1Interface, noAfter time.Time,
logger *zap.SugaredLogger, certsExpire bool) ([]byte, []byte, error) {
interceptorSvcName := os.Getenv(interceptorTLSSvcKey)
interceptorSecretName := os.Getenv(interceptorTLSSecretKey)
namespace := system.Namespace()
Expand All @@ -191,6 +191,19 @@ func createCerts(ctx context.Context, coreV1Interface corev1.CoreV1Interface, no
return []byte{}, []byte{}, err
}

// checking the secret data existence, if secret exist and certs are not expired just return those instead of recreating.
if !certsExpire {
if serverKeyVal, ok := secret.Data[certresources.ServerKey]; ok {
if serverCertVal, ok := secret.Data[certresources.ServerCert]; ok {
if caCertVal, ok := secret.Data[certresources.CACert]; ok {
if string(serverKeyVal) != "" && string(serverCertVal) != "" && string(caCertVal) != "" {
return secret.Data[certresources.ServerCert], secret.Data[certresources.CACert], nil
}
}
}
}
}

serverKey, serverCert, caCert, err := certresources.CreateCerts(ctx, interceptorSvcName, namespace, noAfter)
if err != nil {
logger.Errorf("failed to create certs : %v", err)
Expand Down Expand Up @@ -265,7 +278,7 @@ func (is *Server) checkCertValidity(ctx context.Context, serverCert, caCert []by
if _, err := cert.Verify(opts); err != nil {
logger.Errorf("failed to verify certificate: %v", err.Error())

serverCertNew, caCertNew, err := createCerts(ctx, coreV1Interface, time.Now().Add(Decade), logger)
serverCertNew, caCertNew, err := createCerts(ctx, coreV1Interface, time.Now().Add(Decade), logger, true)
if err != nil {
logger.Errorf("failed to create certs %v", err)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/interceptors/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,13 @@ func Test_SecretNotExist(t *testing.T) {
logger := zaptest.NewLogger(t)
ctx, _ := test.SetupFakeContext(t)
clientSet := fakekubeclient.Get(ctx).CoreV1()
_, _, err := createCerts(ctx, clientSet, time.Now().Add(Decade), logger.Sugar())
_, _, err := createCerts(ctx, clientSet, time.Now().Add(Decade), logger.Sugar(), false)
if err != nil && !strings.Contains(err.Error(), "not found") {
t.Error(err)
}
}

func createSecret(t *testing.T, noAfter time.Time) (v1.CoreV1Interface, []byte, []byte, error) {
func createSecret(t *testing.T, noAfter time.Time, certExpire bool) (v1.CoreV1Interface, []byte, []byte, error) {
t.Setenv(interceptorTLSSvcKey, testsvc)
t.Setenv(interceptorTLSSecretKey, testsecrets)
t.Setenv("SYSTEM_NAMESPACE", testns)
Expand All @@ -236,12 +236,12 @@ func createSecret(t *testing.T, noAfter time.Time) (v1.CoreV1Interface, []byte,
if _, err := clientSet.Secrets(namespace).Create(ctx, localObject, metav1.CreateOptions{}); err != nil {
t.Error(err)
}
sCert, caCert, err := createCerts(ctx, clientSet, noAfter, logger.Sugar())
sCert, caCert, err := createCerts(ctx, clientSet, noAfter, logger.Sugar(), certExpire)
return clientSet, sCert, caCert, err
}

func Test_CreateSecret(t *testing.T) {
_, sCert, caCert, err := createSecret(t, time.Now().Add(Decade))
_, sCert, caCert, err := createSecret(t, time.Now().Add(Decade), true)
if err != nil {
t.Error(err)
}
Expand All @@ -253,7 +253,7 @@ func Test_CreateSecret(t *testing.T) {
func Test_CheckCertValidity(t *testing.T) {
ctx, _ := test.SetupFakeContext(t)
logger := zaptest.NewLogger(t)
clientSet, sCert, caCert, err := createSecret(t, time.Now().Add(second))
clientSet, sCert, caCert, err := createSecret(t, time.Now().Add(second), false)
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -286,7 +286,7 @@ func Test_CheckCertValidity(t *testing.T) {
if err != nil {
t.Error(err)
}
// Making sure that certs expired and and generated one is different than old certs
// Making sure that certs expired and generated one is different than old certs
if string(ciNew1.Spec.ClientConfig.CaBundle) == "" || string(ciNew1.Spec.ClientConfig.CaBundle) == string(ciNew.Spec.ClientConfig.CaBundle) {
t.Error("timeout or failed to regenerate certificate after the certificate expire")
}
Expand Down

0 comments on commit 4543a07

Please sign in to comment.