Skip to content
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

Jump to bottom

Sparse cert creation for kubeadm init #71232

Merged
merged 2 commits into from
Nov 19, 2018
Merged

Sparse cert creation for kubeadm init #71232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e8228f
Move some test functions into test utils
liztio Nov 19, 2018
2f14e18
`kubeadm init` supports sparse certificates
liztio Nov 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/kubeadm/app/cmd/phases/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ go_library(
"//cmd/kubeadm/app/util/apiclient:go_default_library",
"//cmd/kubeadm/app/util/config:go_default_library",
"//cmd/kubeadm/app/util/dryrun:go_default_library",
"//cmd/kubeadm/app/util/pkiutil:go_default_library",
"//pkg/util/normalizer:go_default_library",
"//pkg/version:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand Down Expand Up @@ -72,7 +73,9 @@ go_test(
"//cmd/kubeadm/app/phases/certs:go_default_library",
"//cmd/kubeadm/app/util/pkiutil:go_default_library",
"//cmd/kubeadm/test:go_default_library",
"//cmd/kubeadm/test/certs:go_default_library",
"//pkg/version:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
],
)

Expand Down
26 changes: 20 additions & 6 deletions cmd/kubeadm/app/cmd/phases/certs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
"k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
"k8s.io/kubernetes/pkg/util/normalizer"
)

Expand Down Expand Up @@ -228,9 +229,13 @@ func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error {
return errors.New("certs phase invoked with an invalid data struct")
}

// if external CA mode, skips certificate authority generation
if data.ExternalCA() {
fmt.Printf("[certs] External CA mode: Using existing %s certificate authority\n", ca.BaseName)
// TODO(EKF): can we avoid loading these certificates every time?
if _, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
if _, err := pkiutil.TryLoadKeyFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
fmt.Printf("[certs] Using existing %s certificate authority\n", ca.BaseName)
return nil
}
fmt.Printf("[certs] Using existing %s keyless certificate authority", ca.BaseName)
return nil
}

Expand All @@ -257,9 +262,18 @@ func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert)
return errors.New("certs phase invoked with an invalid data struct")
}

// if external CA mode, skip certificate generation
if data.ExternalCA() {
fmt.Printf("[certs] External CA mode: Using existing %s certificate\n", cert.BaseName)
// TODO(EKF): can we avoid loading these certificates every time?
if certData, _, err := pkiutil.TryLoadCertAndKeyFromDisk(data.CertificateDir(), cert.BaseName); err == nil {
caCertData, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), caCert.BaseName)
if err != nil {
return errors.Wrapf(err, "couldn't load CA certificate %s", caCert.Name)
}

if err := certData.CheckSignatureFrom(caCertData); err != nil {
return errors.Wrapf(err, "[certs] certificate %s not signed by CA certificate %s", cert.BaseName, caCert.BaseName)
}

fmt.Printf("[certs] Using existing %s certificate and key on disk\n", cert.BaseName)
return nil
}

Expand Down
30 changes: 30 additions & 0 deletions cmd/kubeadm/app/cmd/phases/certs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ import (
"os"
"testing"

"github.com/spf13/cobra"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
"k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
"k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
testutil "k8s.io/kubernetes/cmd/kubeadm/test"
certstestutil "k8s.io/kubernetes/cmd/kubeadm/test/certs"
Copy link
Member

Choose a reason for hiding this comment

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

so i just noticed that we are doing this pattern. :[
phases and such are eventually going to be part of the vendorable backend of kubeadm.

cmd/kubeadm/test hold our integration tests it should not to be imported into the "backend", it should be the other way around.

i think the location for such test utils is ideally inside the backend itself, but given we are following a previous pattern this is fine i guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That file could be anywhere, it just happened to be where it was picked. A future PR could very easily move it I think, but that's probably out of scope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed as kubernetes/kubeadm#1257

)

type testCertsData struct {
Expand All @@ -51,6 +53,9 @@ func TestCertsWithCSRs(t *testing.T) {
// global vars
csrOnly = true
csrDir = certDir
defer func() {
csrOnly = false
}()

phase := NewCertsPhase()
// find the api cert phase
Expand All @@ -75,3 +80,28 @@ func TestCertsWithCSRs(t *testing.T) {
t.Fatalf("couldn't load certificate %q: %v", cert.BaseName, err)
}
}

func TestCreateSparseCerts(t *testing.T) {
for _, test := range certstestutil.GetSparseCertTestCases(t) {
t.Run(test.Name, func(t *testing.T) {
tmpdir := testutil.SetupTempDir(t)
defer os.RemoveAll(tmpdir)

certstestutil.WritePKIFiles(t, tmpdir, test.Files)

r := workflow.NewRunner()
r.AppendPhase(NewCertsPhase())
r.SetDataInitializer(func(*cobra.Command) (workflow.RunData, error) {
certsData := &testCertsData{
cfg: testutil.GetDefaultInternalConfig(t),
}
certsData.cfg.CertificatesDir = tmpdir
return certsData, nil
})

if err := r.Run(); (err != nil) != test.ExpectError {
t.Fatalf("expected error to be %t, got %t (%v)", test.ExpectError, (err != nil), err)
}
})
}
}