Skip to content

installer/pkg/config-generator/tls: add kube + tnc tls certs #22

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.

Sign up for GitHub

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

Merged
trawler merged 3 commits into from
Jul 9, 2018
Merged

installer/pkg/config-generator/tls: add kube + tnc tls certs #22

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82bc004
installer/pkg/config-generator/tls: add kube tls certs
Jul 6, 2018
50182d7
installer/pkg/config-generator/tls: tnc certs
Jul 6, 2018
26930f2
post-review fixes
Jul 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 109 additions & 9 deletions installer/pkg/config-generator/tls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,41 @@ import (
"encoding/pem"
"fmt"
"io/ioutil"
"net"
"path/filepath"
"time"

"github.com/openshift/installer/installer/pkg/tls"
)

const (
adminCertPath = "generated/newTLS/admin.crt"
adminKeyPath = "generated/newTLS/admin.key"
aggregatorCACertPath = "generated/newTLS/aggregator-ca.crt"
aggregatorCAKeyPath = "generated/newTLS/aggregator-ca.key"
apiServerCertPath = "generated/newTLS/apiserver.crt"
apiServerKeyPath = "generated/newTLS/apiserver.key"
apiServerProxyCertPath = "generated/newTLS/apiserver-proxy.crt"
apiServerProxyKeyPath = "generated/newTLS/apiserver-proxy.key"
etcdCACertPath = "generated/newTLS/etcd-ca.crt"
etcdCAKeyPath = "generated/newTLS/etcd-ca.key"
etcdClientCertPath = "generated/newTLS/etcd-client.crt"
etcdClientKeyPath = "generated/newTLS/etcd-client.key"
ingressCACertPath = "generated/newTLS/ingress-ca.crt"
ingressCertPath = "generated/newTLS/ingress.crt"
ingressKeyPath = "generated/newTLS/ingress.key"
kubeCACertPath = "generated/newTLS/kube-ca.crt"
kubeCAKeyPath = "generated/newTLS/kube-ca.key"
kubeletCertPath = "generated/newTLS/kubelet.crt"
kubeletKeyPath = "generated/newTLS/kubelet.key"
osAPIServerCertPath = "generated/newTLS/openshift-apiserver.crt"
osAPIServerKeyPath = "generated/newTLS/openshift-apiserver.key"
rootCACertPath = "generated/newTLS/root-ca.crt"
rootCAKeyPath = "generated/newTLS/root-ca.key"
serviceServiceCACertPath = "generated/newTLS/service-serving-ca.crt"
serviceServiceCAKeyPath = "generated/newTLS/service-serving-ca.key"
ingressCACertPath = "generated/newTLS/ingress-ca.crt"
ingressCertPath = "generated/newTLS/ingress.crt"
ingressKeyPath = "generated/newTLS/ingress.key"
serviceServingCACertPath = "generated/newTLS/service-serving-ca.crt"
serviceServingCAKeyPath = "generated/newTLS/service-serving-ca.key"
tncCertPath = "generated/newTLS/tnc.crt"
tncKeyPath = "generated/newTLS/tnc.key"

validityThreeYears = time.Hour * 24 * 365 * 3
)
Expand Down Expand Up @@ -105,7 +118,7 @@ func (c *ConfigGenerator) GenerateTLSConfig(clusterDir string) error {
Validity: validityThreeYears,
IsCA: true,
}
if _, _, err := generateCert(clusterDir, caKey, caCert, serviceServiceCAKeyPath, serviceServiceCACertPath, cfg); err != nil {
if _, _, err := generateCert(clusterDir, caKey, caCert, serviceServingCAKeyPath, serviceServingCACertPath, cfg); err != nil {
return fmt.Errorf("failed to generate service-serving CA: %v", err)
}

Expand All @@ -123,8 +136,95 @@ func (c *ConfigGenerator) GenerateTLSConfig(clusterDir string) error {
Validity: validityThreeYears,
IsCA: false}

if _, _, err := generateCert(clusterDir, kubeCAKey, kubeCACert, ingressKeyPath, ingressCertPath, cfg); err != nil {
return fmt.Errorf("failed to generate ingress CAs: %v", err)
if _, _, err = generateCert(clusterDir, kubeCAKey, kubeCACert, ingressKeyPath, ingressCertPath, cfg); err != nil {

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove the : from here? we can happily keep it like we do in the rest of the instances above

return fmt.Errorf("failed to generate ingress CA: %v", err)
}

// Kube admin certs
cfg = &tls.CertCfg{
KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
Subject: pkix.Name{CommonName: "system:admin", Organization: []string{"system:masters"}},
Validity: validityThreeYears,
IsCA: false}

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's convention to terminate the curly brace on a new line


if _, _, err = generateCert(clusterDir, kubeCAKey, kubeCACert, adminKeyPath, adminCertPath, cfg); err != nil {

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing about the : here

return fmt.Errorf("failed to generate kube admin certificate: %v", err)
}

// Kube API server certs
apiServerAddress, err := cidrhost(c.Cluster.Networking.ServiceCIDR, 1)
if err != nil {
return fmt.Errorf("can't resolve api server host address: %v", err)
}
cfg = &tls.CertCfg{
KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
Subject: pkix.Name{CommonName: "kube-apiserver", Organization: []string{"kube-master"}},
DNSNames: []string{fmt.Sprintf("%s-%s.%s", c.Name, "api", c.BaseDomain), "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local"},

@wking wking Jul 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using %s to format the hard-coded api string, why not include that in the template?

fmt.Sprintf("%s-api.%s", c.Name, c.BaseDomain)

For precedent on this approach, see here, here, and likely other places. If the pattern sounds appealing, there are some other fmt.Sprintf calls in your PR that could also be updated.

@trawler trawler Jul 9, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point 👍

Validity: validityThreeYears,
IPAddresses: []net.IP{net.ParseIP(apiServerAddress)},
IsCA: false}

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same note about the closing curly brace here


if _, _, err := generateCert(clusterDir, kubeCAKey, kubeCACert, apiServerKeyPath, apiServerCertPath, cfg); err != nil {
return fmt.Errorf("failed to generate kube api server certificate: %v", err)
}

// Kube API openshift certs
cfg = &tls.CertCfg{
KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
Subject: pkix.Name{CommonName: "openshift-apiserver", Organization: []string{"kube-master"}},
DNSNames: []string{
fmt.Sprintf("%s-%s.%s", c.Name, "api", c.BaseDomain),
"openshift-apiserver",
"openshift-apiserver.kube-system",
"openshift-apiserver.kube-system.svc",
"openshift-apiserver.kube-system.svc.cluster.local",
"localhost", "127.0.0.1"},
Validity: validityThreeYears,
IPAddresses: []net.IP{net.ParseIP(apiServerAddress)},
IsCA: false}

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same again here


if _, _, err := generateCert(clusterDir, kubeCAKey, kubeCACert, osAPIServerKeyPath, osAPIServerCertPath, cfg); err != nil {
return fmt.Errorf("failed to generate kube openshift server certificate: %v", err)

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is kube openshift server? make it should be openshift API server?

}

// Kube API proxy certs
cfg = &tls.CertCfg{
KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
Subject: pkix.Name{CommonName: "kube-apiserver-proxy", Organization: []string{"kube-master"}},
Validity: validityThreeYears,
IsCA: false}

if _, _, err := generateCert(clusterDir, kubeCAKey, kubeCACert, apiServerProxyKeyPath, apiServerProxyCertPath, cfg); err != nil {
return fmt.Errorf("failed to generate kube api proxy certificate: %v", err)
}

// Kubelet certs
cfg = &tls.CertCfg{
KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
Subject: pkix.Name{CommonName: "system:serviceaccount:kube-system:default", Organization: []string{"system:serviceaccounts:kube-system"}},
Validity: validityThreeYears,
IsCA: false}

if _, _, err := generateCert(clusterDir, kubeCAKey, kubeCACert, kubeletKeyPath, kubeletCertPath, cfg); err != nil {
return fmt.Errorf("failed to generate kubelet certificate: %v", err)
}

// TNC certs
tncDomain := fmt.Sprintf("%s-%s.%s", c.Name, "tnc", c.BaseDomain)
cfg = &tls.CertCfg{
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
DNSNames: []string{tncDomain},
Subject: pkix.Name{CommonName: tncDomain},
Validity: validityThreeYears,
IsCA: false}

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same note about the closing curly brace


if _, _, err = generateCert(clusterDir, caKey, caCert, tncKeyPath, tncCertPath, cfg); err != nil {

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same note about : here.

return fmt.Errorf("failed to generate tnc certs: %v", err)

@squat squat Jul 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/certs/certificate/

}
return nil
}
Expand Down Expand Up @@ -206,7 +306,7 @@ func generateCert(clusterDir string,
}

// create a CSR
csrTmpl := x509.CertificateRequest{Subject: cfg.Subject, DNSNames: cfg.DNSNames}
csrTmpl := x509.CertificateRequest{Subject: cfg.Subject, DNSNames: cfg.DNSNames, IPAddresses: cfg.IPAddresses}
csrBytes, err := x509.CreateCertificateRequest(rand.Reader, &csrTmpl, key)
if err != nil {
return nil, nil, fmt.Errorf("error creating certificate request: %v", err)
Expand Down