Skip to content

Commit

Permalink
Merge pull request #124126 from carlory/automated-cherry-pick-of-#124…
Browse files Browse the repository at this point in the history
…124-upstream-release-1.29

Automated cherry pick of #124124: fix panic with SIGSEGV in kubeadm certs check-expiration
  • Loading branch information
k8s-ci-robot committed Apr 5, 2024
2 parents 63d93a5 + aa6c135 commit ac645d2
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 9 deletions.
4 changes: 2 additions & 2 deletions cmd/kubeadm/app/phases/certs/renewal/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func (rm *Manager) CertificateExists(name string) (bool, error) {
return false, errors.Errorf("%s is not a known certificate", name)
}

return handler.readwriter.Exists(), nil
return handler.readwriter.Exists()
}

// GetCertificateExpirationInfo returns certificate expiration info.
Expand Down Expand Up @@ -358,7 +358,7 @@ func (rm *Manager) CAExists(name string) (bool, error) {
return false, errors.Errorf("%s is not a known certificate", name)
}

return handler.readwriter.Exists(), nil
return handler.readwriter.Exists()
}

// GetCAExpirationInfo returns CA expiration info.
Expand Down
17 changes: 10 additions & 7 deletions cmd/kubeadm/app/phases/certs/renewal/readwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
// read or write a certificate stored/embedded in a file
type certificateReadWriter interface {
//Exists return true if the certificate exists
Exists() bool
Exists() (bool, error)

// Read a certificate stored/embedded in a file
Read() (*x509.Certificate, error)
Expand All @@ -61,17 +61,20 @@ func newPKICertificateReadWriter(certificateDir string, baseName string) *pkiCer
}

// Exists checks if a certificate exist
func (rw *pkiCertificateReadWriter) Exists() bool {
func (rw *pkiCertificateReadWriter) Exists() (bool, error) {
certificatePath, _ := pkiutil.PathsForCertAndKey(rw.certificateDir, rw.baseName)
return fileExists(certificatePath)
}

func fileExists(filename string) bool {
func fileExists(filename string) (bool, error) {
info, err := os.Stat(filename)
if os.IsNotExist(err) {
return false
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
return !info.IsDir()
return !info.IsDir(), nil
}

// Read a certificate from a file the K8s pki managed by kubeadm
Expand Down Expand Up @@ -120,7 +123,7 @@ func newKubeconfigReadWriter(kubernetesDir string, kubeConfigFileName string, ce
}

// Exists checks if a certificate embedded in kubeConfig file exists
func (rw *kubeConfigReadWriter) Exists() bool {
func (rw *kubeConfigReadWriter) Exists() (bool, error) {
return fileExists(rw.kubeConfigFilePath)
}

Expand Down
145 changes: 145 additions & 0 deletions cmd/kubeadm/app/phases/certs/renewal/readwriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package renewal
import (
"crypto"
"crypto/x509"
"fmt"
"net"
"os"
"path/filepath"
Expand Down Expand Up @@ -199,3 +200,147 @@ func writeTestKubeconfig(t *testing.T, dir, name string, caCert *x509.Certificat

return cert
}

func TestFileExists(t *testing.T) {
tmpdir, err := os.MkdirTemp("", "")
if err != nil {
t.Fatalf("Couldn't create tmpdir: %v", err)
}
defer func() {
err = os.RemoveAll(tmpdir)
if err != nil {
t.Fatalf("Fail to remove tmpdir: %v", err)
}
}()
tmpfile, err := os.CreateTemp(tmpdir, "")
if err != nil {
t.Fatalf("Couldn't create tmpfile: %v", err)
}
tests := []struct {
name string
filename string
want bool
}{
{
name: "file exist",
filename: tmpfile.Name(),
want: true,
},
{
name: "file does not exist",
filename: "foo",
want: false,
},
{
name: "file path is a dir",
filename: tmpdir,
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got, _ := fileExists(tt.filename); got != tt.want {
t.Errorf("fileExists() = %v, want %v", got, tt.want)
}
})
}
}

func TestPKICertificateReadWriterExists(t *testing.T) {
tmpdir, err := os.MkdirTemp("", "")
if err != nil {
t.Fatalf("Couldn't create tmpdir: %v", err)
}
defer func() {
err = os.RemoveAll(tmpdir)
if err != nil {
t.Fatalf("Fail to remove tmpdir: %v", err)
}
}()
filename := "testfile"
tmpfilepath := filepath.Join(tmpdir, fmt.Sprintf(filename+".crt"))
err = os.WriteFile(tmpfilepath, nil, 0644)
if err != nil {
t.Fatalf("Couldn't write file: %v", err)
}
type fields struct {
baseName string
certificateDir string
}
tests := []struct {
name string
fields fields
want bool
}{
{
name: "cert file exists",
fields: fields{
baseName: filename,
certificateDir: tmpdir,
},
want: true,
},
{
name: "cert file does not exist",
fields: fields{
baseName: "foo",
certificateDir: tmpdir,
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rw := &pkiCertificateReadWriter{
baseName: tt.fields.baseName,
certificateDir: tt.fields.certificateDir,
}
if got, _ := rw.Exists(); got != tt.want {
t.Errorf("pkiCertificateReadWriter.Exists() = %v, want %v", got, tt.want)
}
})
}
}

func TestKubeConfigReadWriterExists(t *testing.T) {
tmpdir, err := os.MkdirTemp("", "")
if err != nil {
t.Fatalf("Couldn't create tmpdir: %v", err)
}
defer func() {
err = os.RemoveAll(tmpdir)
if err != nil {
t.Fatalf("Fail to remove tmpdir: %v", err)
}
}()
tmpfile, err := os.CreateTemp(tmpdir, "")
if err != nil {
t.Fatalf("Couldn't create tmpfile: %v", err)
}
tests := []struct {
name string
kubeConfigFilePath string
want bool
}{
{
name: "file exists",
kubeConfigFilePath: tmpfile.Name(),
want: true,
},
{
name: "file does not exist",
kubeConfigFilePath: "foo",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rw := &kubeConfigReadWriter{
kubeConfigFilePath: tt.kubeConfigFilePath,
}
if got, _ := rw.Exists(); got != tt.want {
t.Errorf("kubeConfigReadWriter.Exists() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit ac645d2

Please sign in to comment.