Skip to content

Commit

Permalink
use Knative certificates for Serving encryption
Browse files Browse the repository at this point in the history
  • Loading branch information
ReToCode committed Nov 22, 2023
1 parent f0c9ff1 commit 6198bf3
Show file tree
Hide file tree
Showing 51 changed files with 1,332 additions and 1,428 deletions.
30 changes: 15 additions & 15 deletions .github/workflows/kind-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,18 @@ jobs:
fail-fast: false # Keep running if one leg fails.
matrix:
k8s-version:
- v1.26.x
- v1.27.x
# - v1.26.x
# - v1.27.x
- v1.28.x

ingress:
- kourier
- kourier-tls
- istio
- istio-tls
# - istio
# - istio-tls
# Disabled due to flakiness: https://github.com/knative/serving/issues/14637
# - istio-ambient
- contour
# - contour
# Disabled due to consistent failures
# - gateway_istio

Expand All @@ -114,16 +114,16 @@ jobs:
# test-flags: -enable-alpha
# namespace-resources: httproute

- ingress: contour
namespace-resources: httpproxy

- ingress: istio
namespace-resources: virtualservices

- ingress: istio-tls
ingress-class: istio
namespace-resources: virtualservices
enable-tls: 1
# - ingress: contour
# namespace-resources: httpproxy
#
# - ingress: istio
# namespace-resources: virtualservices
#
# - ingress: istio-tls
# ingress-class: istio
# namespace-resources: virtualservices
# enable-tls: 1

# Disabled due to flakiness: https://github.com/knative/serving/issues/14637
# - ingress: istio-ambient
Expand Down
7 changes: 5 additions & 2 deletions cmd/activator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,11 @@ func main() {
// At this moment activator with TLS does not disable HTTP.
// See also https://github.com/knative/serving/issues/12808.
if tlsEnabled {
logger.Info("Knative Internal TLS is enabled")
certCache = certificate.NewCertCache(ctx)
logger.Info("Knative system-internal-tls is enabled")
certCache, err = certificate.NewCertCache(ctx)
if err != nil {
logger.Fatalw("Failed to create certificate cache", zap.Error(err))
}
transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, certCache.TLSContext())
}

Expand Down
13 changes: 1 addition & 12 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
// The set of controllers this controller process runs.
"flag"

certificate "knative.dev/networking/pkg/certificates/reconciler"
"knative.dev/pkg/reconciler"
"knative.dev/pkg/signals"
"knative.dev/serving/pkg/reconciler/configuration"
Expand All @@ -32,18 +31,11 @@ import (
"knative.dev/serving/pkg/reconciler/serverlessservice"
"knative.dev/serving/pkg/reconciler/service"

// This defines the shared main for injected controllers.
filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
"knative.dev/pkg/injection"
"knative.dev/pkg/injection/sharedmain"
"knative.dev/serving/pkg/networking"
"knative.dev/serving/pkg/reconciler/domainmapping"
)

const (
secretLabelNamePostfix = "-ctrl"
)

var ctors = []injection.ControllerConstructor{
configuration.NewController,
labeler.NewController,
Expand All @@ -53,7 +45,6 @@ var ctors = []injection.ControllerConstructor{
service.NewController,
gc.NewController,
nscert.NewController,
certificate.NewControllerFactory(networking.ServingCertName),
domainmapping.NewController,
}

Expand All @@ -62,7 +53,5 @@ func main() {
"reconciliation-timeout", reconciler.DefaultTimeout,
"The amount of time to give each reconciliation of a resource to complete before its context is canceled.")

labelName := networking.ServingCertName + secretLabelNamePostfix
ctx := filteredFactory.WithSelectors(signals.NewContext(), labelName)
sharedmain.MainWithContext(ctx, "controller", ctors...)
sharedmain.MainWithContext(signals.NewContext(), "controller", ctors...)
}
14 changes: 14 additions & 0 deletions config/core/300-knativecertificate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: networking.internal.knative.dev/v1alpha1
kind: Certificate
metadata:
annotations:
networking.knative.dev/certificate.class: cert-manager.certificate.networking.knative.dev
labels:
networking.knative.dev/certificate-type: system-internal
name: routing-serving-certs
namespace: knative-serving
spec:
dnsNames:
- kn-routing
- data-plane.knative.dev # for reverse-compatibility with net-* implementations that do not work with multi-SANs
secretName: routing-serving-certs
47 changes: 0 additions & 47 deletions config/core/300-secret.yaml

This file was deleted.

2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,5 @@ require (

// TODO: https://github.com/knative/serving/issues/14597
replace github.com/gorilla/websocket => github.com/gorilla/websocket v1.5.0

replace knative.dev/networking => /Users/rlehmann/code/knative/networking
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -948,8 +948,6 @@ knative.dev/caching v0.0.0-20231109234001-c2b2543ace4a h1:DI0ZxUDWNFQbHUZYqTdR7d
knative.dev/caching v0.0.0-20231109234001-c2b2543ace4a/go.mod h1:BlOGe4qgf8fOd0CziFWnblXFbwxqeRK3cBdPdzXj8kM=
knative.dev/hack v0.0.0-20231109190034-5deaddeb51a7 h1:HXf7M7n9jwn+Hp904r0HXRSymf+DLXSciFpXVpCg+Bs=
knative.dev/hack v0.0.0-20231109190034-5deaddeb51a7/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
knative.dev/networking v0.0.0-20231109233957-8f3c5211035b h1:xdj40UcZBX4QeZiOOuCZDDGLM7gB+Wg07AKT7KVFlD0=
knative.dev/networking v0.0.0-20231109233957-8f3c5211035b/go.mod h1:h98yk6yX/fCKx0t1uDNsA0zD1QM2k97P59/XMwOe944=
knative.dev/pkg v0.0.0-20231115001034-97c7258e3a98 h1:uvOLwp5Ar7oJlaYEszh51CemuZc1sRRI14xzKhUEF3U=
knative.dev/pkg v0.0.0-20231115001034-97c7258e3a98/go.mod h1:56Qcm0ai7xPWqGxpOnjRi4sAX9fZM9UDTk7fKyjUqZM=
pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw=
Expand Down
11 changes: 6 additions & 5 deletions pkg/activator/certificate/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
"sync"

"go.uber.org/zap"
Expand All @@ -47,8 +48,8 @@ type CertCache struct {
certificatesMux sync.RWMutex
}

// NewCertCache starts secretInformer.
func NewCertCache(ctx context.Context) *CertCache {
// NewCertCache creates and starts the certificate cache that watches Activators certificate.
func NewCertCache(ctx context.Context) (*CertCache, error) {
secretInformer := secretinformer.Get(ctx)

cr := &CertCache{
Expand All @@ -58,8 +59,8 @@ func NewCertCache(ctx context.Context) *CertCache {

secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingRoutingCertName)
if err != nil {
cr.logger.Warnw("failed to get secret", zap.Error(err))
return nil
return nil, fmt.Errorf("failed to get activator certificate, secret %s/%s was not found: %w. Enabling system-internal-tls requires the secret to be present and populated with a valid certificate and CA",
system.Namespace(), netcfg.ServingRoutingCertName, err)
}

cr.updateCache(secret)
Expand All @@ -72,7 +73,7 @@ func NewCertCache(ctx context.Context) *CertCache {
},
})

return cr
return cr, nil
}

func (cr *CertCache) handleCertificateAdd(added interface{}) {
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/serving/v1/route_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,6 @@ const (
// RouteConditionCertificateProvisioned condition when it is set to True
// because external-domain-tls was not enabled.
ExternalDomainTLSNotEnabledMessage = "external-domain-tls is not enabled"

// TLSNotEnabledForClusterLocalMessage is the message which is set on the
// RouteConditionCertificateProvisioned condition when it is set to True
// because the domain is cluster-local.
TLSNotEnabledForClusterLocalMessage = "TLS is not enabled for cluster-local"
)

// MarkTLSNotEnabled sets RouteConditionCertificateProvisioned to true when
Expand Down
41 changes: 34 additions & 7 deletions pkg/queue/certificate/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ limitations under the License.
package certificate

import (
"bytes"
"context"
"crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/big"
"os"
"testing"
"time"
Expand Down Expand Up @@ -94,26 +100,47 @@ func TestCertificateRotation(t *testing.T) {
}

func createAndSaveCertificate(san, dir string) error {
ca, err := certificates.CreateCACerts(1 * time.Hour)
if err != nil {
return err
cert := &x509.Certificate{
SerialNumber: big.NewInt(2019),
Subject: pkix.Name{
Organization: []string{"Knative"},
},
NotBefore: time.Now(),
NotAfter: time.Now().AddDate(10, 0, 0),
IsCA: false,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
BasicConstraintsValid: true,
}
cert.DNSNames = []string{san}

caCert, caKey, err := ca.Parse()
pk, err := rsa.GenerateKey(rand.Reader, 4096)
if err != nil {
return err
}

cert, err := certificates.CreateCert(caKey, caCert, 1*time.Hour, san)
certBytes, err := x509.CreateCertificate(rand.Reader, cert, cert, &pk.PublicKey, pk)
if err != nil {
return err
}

if err := os.WriteFile(dir+"/"+certificates.CertName, cert.CertBytes(), 0644); err != nil {
caPEM := new(bytes.Buffer)
pem.Encode(caPEM, &pem.Block{
Type: "CERTIFICATE",
Bytes: certBytes,
})

caPrivKeyPEM := new(bytes.Buffer)
pem.Encode(caPrivKeyPEM, &pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: x509.MarshalPKCS1PrivateKey(pk),
})

if err := os.WriteFile(dir+"/"+certificates.CertName, caPEM.Bytes(), 0644); err != nil {
return err
}

return os.WriteFile(dir+"/"+certificates.PrivateKeyName, cert.PrivateKeyBytes(), 0644)
return os.WriteFile(dir+"/"+certificates.PrivateKeyName, caPrivKeyPEM.Bytes(), 0644)
}

func getSAN(c *tls.Certificate) (string, error) {
Expand Down
7 changes: 6 additions & 1 deletion pkg/reconciler/accessor/networking/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,15 @@ func ReconcileCertificate(ctx context.Context, owner kmeta.Accessor, desired *v1
return nil, kaccessor.NewAccessorError(
fmt.Errorf("owner: %s with Type %T does not own Certificate: %q", owner.GetName(), owner, cert.Name),
kaccessor.NotOwnResource)
} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) {
} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) ||
!equality.Semantic.DeepEqual(cert.Annotations, desired.Annotations) ||
!equality.Semantic.DeepEqual(cert.Labels, desired.Labels) {

// Don't modify the informers copy
existing := cert.DeepCopy()
existing.Spec = desired.Spec
existing.Annotations = desired.Annotations
existing.Labels = desired.Labels
cert, err = certAccessor.GetNetworkingClient().NetworkingV1alpha1().Certificates(existing.Namespace).Update(ctx, existing, metav1.UpdateOptions{})
if err != nil {
recorder.Eventf(owner, corev1.EventTypeWarning, "UpdateFailed",
Expand Down
7 changes: 5 additions & 2 deletions pkg/reconciler/domainmapping/resources/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/networking/pkg/config"

"knative.dev/networking/pkg/apis/networking"
networkingv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
Expand Down Expand Up @@ -58,7 +59,8 @@ func TestMakeCertificate(t *testing.T) {
Namespace: "the-namespace",
Annotations: map[string]string{networking.CertificateClassAnnotationKey: certClass},
Labels: map[string]string{
serving.DomainMappingUIDLabelKey: "mapping.com",
serving.DomainMappingUIDLabelKey: "mapping.com",
networking.CertificateTypeLabelKey: string(config.CertificateExternalDomain),
},
},
Spec: networkingv1alpha1.CertificateSpec{
Expand Down Expand Up @@ -96,7 +98,8 @@ func TestMakeCertificate(t *testing.T) {
"others": "kept",
},
Labels: map[string]string{
serving.DomainMappingUIDLabelKey: "mapping.com",
serving.DomainMappingUIDLabelKey: "mapping.com",
networking.CertificateTypeLabelKey: string(config.CertificateExternalDomain),
},
},
Spec: networkingv1alpha1.CertificateSpec{
Expand Down
2 changes: 2 additions & 0 deletions pkg/reconciler/domainmapping/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
},
Labels: map[string]string{
serving.DomainMappingUIDLabelKey: "becomes.ready.run",
netapi.CertificateTypeLabelKey: string(netcfg.CertificateExternalDomain),
},
},
Spec: netv1alpha1.CertificateSpec{
Expand Down Expand Up @@ -1114,6 +1115,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
},
Labels: map[string]string{
serving.DomainMappingUIDLabelKey: "challenged.com",
netapi.CertificateTypeLabelKey: string(netcfg.CertificateExternalDomain),
},
},
Spec: netv1alpha1.CertificateSpec{
Expand Down

0 comments on commit 6198bf3

Please sign in to comment.