-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate net-certmanager in Serving #15066
Changes from all commits
49b48b3
bfe3d4b
06b8a33
33fab2d
b08d312
221cf19
f5c2333
6a23975
502c67c
3de96d8
a279ff7
740f03a
04805a8
049ac7b
177f361
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,25 @@ limitations under the License. | |
package main | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
// The set of controllers this controller process runs. | ||
"flag" | ||
"log" | ||
|
||
v1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/client-go/kubernetes" | ||
netcfg "knative.dev/networking/pkg/config" | ||
"knative.dev/pkg/injection" | ||
"knative.dev/pkg/injection/sharedmain" | ||
"knative.dev/pkg/reconciler" | ||
"knative.dev/pkg/signals" | ||
"knative.dev/pkg/system" | ||
"knative.dev/serving/pkg/reconciler/certificate" | ||
"knative.dev/serving/pkg/reconciler/configuration" | ||
"knative.dev/serving/pkg/reconciler/domainmapping" | ||
"knative.dev/serving/pkg/reconciler/gc" | ||
"knative.dev/serving/pkg/reconciler/labeler" | ||
"knative.dev/serving/pkg/reconciler/nscert" | ||
|
@@ -31,9 +44,12 @@ import ( | |
"knative.dev/serving/pkg/reconciler/serverlessservice" | ||
"knative.dev/serving/pkg/reconciler/service" | ||
|
||
"knative.dev/pkg/injection" | ||
"knative.dev/pkg/injection/sharedmain" | ||
"knative.dev/serving/pkg/reconciler/domainmapping" | ||
versioned "knative.dev/serving/pkg/client/certmanager/clientset/versioned" | ||
"knative.dev/serving/pkg/client/certmanager/injection/informers/acme/v1/challenge" | ||
v1certificate "knative.dev/serving/pkg/client/certmanager/injection/informers/certmanager/v1/certificate" | ||
"knative.dev/serving/pkg/client/certmanager/injection/informers/certmanager/v1/certificaterequest" | ||
"knative.dev/serving/pkg/client/certmanager/injection/informers/certmanager/v1/clusterissuer" | ||
"knative.dev/serving/pkg/client/certmanager/injection/informers/certmanager/v1/issuer" | ||
) | ||
|
||
var ctors = []injection.ControllerConstructor{ | ||
|
@@ -53,5 +69,68 @@ func main() { | |
"reconciliation-timeout", reconciler.DefaultTimeout, | ||
"The amount of time to give each reconciliation of a resource to complete before its context is canceled.") | ||
|
||
sharedmain.MainWithContext(signals.NewContext(), "controller", ctors...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have a second method in sharedmain where we pass a function to do:
to avoid duplicating code here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or we just iterate on an array of informers. Moving to sharedmain should be the next iteration imho. |
||
ctx := signals.NewContext() | ||
|
||
// HACK: This parses flags, so the above should be set once this runs. | ||
cfg := injection.ParseAndGetRESTConfigOrDie() | ||
|
||
// If nil it panics | ||
client := kubernetes.NewForConfigOrDie(cfg) | ||
|
||
if shouldEnableNetCertManagerController(ctx, client) { | ||
v := versioned.NewForConfigOrDie(cfg) | ||
if ok, err := certManagerCRDsExist(v); !ok { | ||
log.Fatalf("Please install cert-manager: %v", err) | ||
} | ||
for _, inf := range []injection.InformerInjector{challenge.WithInformer, v1certificate.WithInformer, certificaterequest.WithInformer, clusterissuer.WithInformer, issuer.WithInformer} { | ||
injection.Default.RegisterInformer(inf) | ||
} | ||
ctors = append(ctors, certificate.NewController) | ||
} | ||
|
||
sharedmain.MainWithConfig(ctx, "controller", cfg, ctors...) | ||
} | ||
|
||
func shouldEnableNetCertManagerController(ctx context.Context, client *kubernetes.Clientset) bool { | ||
var cm *v1.ConfigMap | ||
var err error | ||
if cm, err = client.CoreV1().ConfigMaps(system.Namespace()).Get(ctx, "config-network", metav1.GetOptions{}); err != nil { | ||
log.Fatalf("Failed to get cm config-network: %v", err) | ||
} | ||
netCfg, err := netcfg.NewConfigFromMap(cm.Data) | ||
if err != nil { | ||
log.Fatalf("Failed to construct network config: %v", err) | ||
} | ||
|
||
return netCfg.ExternalDomainTLS || netCfg.SystemInternalTLSEnabled() || (netCfg.ClusterLocalDomainTLS == netcfg.EncryptionEnabled) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should validate cert manager CRDs as installed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can do that but also the user could know via docs that internal encryption requires cert manager. Personally I think it is a bit too much but I can add it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
netCfg.NamespaceWildcardCertSelector != nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can |
||
} | ||
|
||
func certManagerCRDsExist(client *versioned.Clientset) (bool, error) { | ||
if ok, err := findCRD(client, "cert-manager.io/v1", []string{"certificaterequests", "certificates", "clusterissuers", "issuers"}); !ok { | ||
return false, err | ||
} | ||
if ok, err := findCRD(client, "acme.cert-manager.io/v1", []string{"challenges"}); !ok { | ||
return false, err | ||
} | ||
return true, nil | ||
} | ||
|
||
func findCRD(client *versioned.Clientset, groupVersion string, crds []string) (bool, error) { | ||
resourceList, err := client.Discovery().ServerResourcesForGroupVersion(groupVersion) | ||
if err != nil { | ||
return false, err | ||
} | ||
for _, crdName := range crds { | ||
isCRDPresent := false | ||
for _, resource := range resourceList.APIResources { | ||
if resource.Name == crdName { | ||
isCRDPresent = true | ||
} | ||
} | ||
if !isCRDPresent { | ||
return false, fmt.Errorf("cert manager crds are missing: %s", crdName) | ||
} | ||
} | ||
return true, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
# Copyright 2020 The Knative Authors | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
apiVersion: v1 | ||
kind: ConfigMap | ||
metadata: | ||
name: config-certmanager | ||
namespace: knative-serving | ||
labels: | ||
app.kubernetes.io/name: knative-serving | ||
app.kubernetes.io/component: controller | ||
app.kubernetes.io/version: devel | ||
networking.knative.dev/certificate-provider: cert-manager | ||
annotations: | ||
knative.dev/example-checksum: "b7a9a602" | ||
data: | ||
_example: | | ||
################################ | ||
# # | ||
# EXAMPLE CONFIGURATION # | ||
# # | ||
################################ | ||
|
||
# This block is not actually functional configuration, | ||
# but serves to illustrate the available configuration | ||
# options and document them in a way that is accessible | ||
# to users that `kubectl edit` this config map. | ||
# | ||
# These sample configuration options may be copied out of | ||
# this block and unindented to actually change the configuration. | ||
|
||
# issuerRef is a reference to the issuer for external-domain certificates used for ingress. | ||
# IssuerRef should be either `ClusterIssuer` or `Issuer`. | ||
# Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/ | ||
# for more details about IssuerRef configuration. | ||
# If the issuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used. | ||
issuerRef: | | ||
kind: ClusterIssuer | ||
name: letsencrypt-issuer | ||
|
||
# clusterLocalIssuerRef is a reference to the issuer for cluster-local-domain certificates used for ingress. | ||
# clusterLocalIssuerRef should be either `ClusterIssuer` or `Issuer`. | ||
# Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/ | ||
# for more details about ClusterInternalIssuerRef configuration. | ||
# If the clusterLocalIssuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used. | ||
clusterLocalIssuerRef: | | ||
kind: ClusterIssuer | ||
name: your-company-issuer | ||
|
||
# systemInternalIssuerRef is a reference to the issuer for certificates for system-internal-tls certificates used by Knative internal components. | ||
# systemInternalIssuerRef should be either `ClusterIssuer` or `Issuer`. | ||
# Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/ | ||
# for more details about ClusterInternalIssuerRef configuration. | ||
# If the systemInternalIssuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used. | ||
systemInternalIssuerRef: | | ||
kind: ClusterIssuer | ||
name: knative-selfsigned-issuer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted it so I can test. Tests seem to pass here consistently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#15052 should also help.