Skip to content

Commit

Permalink
config: handle Kube API CA certificate as data, not a file path
Browse files Browse the repository at this point in the history 
We need to pass the CA data itself between ovnkube-node and the cnishim
since the node is containerized and the shim is not, and the path could
be different between the two since they have different filesystem namespaces.

So we might as well just read the CA file and pass data around internally,
rather than using a file path.

Signed-off-by: Dan Williams <dcbw@redhat.com>
  • Loading branch information
@dcbw @trozet
dcbw authored and trozet committed Aug 24, 2021
1 parent 2171779 commit 7156032
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 40 deletions.
10 changes: 8 additions & 2 deletions go-controller/pkg/config/config.go
View file
Expand Up @@ -231,6 +231,7 @@ type CNIConfig struct {
type KubernetesConfig struct {
Kubeconfig string `gcfg:"kubeconfig"`
CACert string `gcfg:"cacert"`
CAData []byte
APIServer string `gcfg:"apiserver"`
Token string `gcfg:"token"`
CompatServiceCIDR string `gcfg:"service-cidr"`
Expand Down Expand Up @@ -1145,8 +1146,13 @@ func buildKubernetesConfig(exec kexec.Interface, cli, file *config, saPath strin
if Kubernetes.Kubeconfig != "" && !pathExists(Kubernetes.Kubeconfig) {
return fmt.Errorf("kubernetes kubeconfig file %q not found", Kubernetes.Kubeconfig)
}
if Kubernetes.CACert != "" && !pathExists(Kubernetes.CACert) {
return fmt.Errorf("kubernetes CA certificate file %q not found", Kubernetes.CACert)

if Kubernetes.CACert != "" {
bytes, err := ioutil.ReadFile(Kubernetes.CACert)
if err != nil {
return err
}
Kubernetes.CAData = bytes
}

url, err := url.Parse(Kubernetes.APIServer)
Expand Down
60 changes: 35 additions & 25 deletions go-controller/pkg/config/config_test.go
View file
Expand Up @@ -113,12 +113,13 @@ var _ = AfterSuite(func() {
gomega.Expect(err).NotTo(gomega.HaveOccurred())
})

func createTempFile(name string) (string, error) {
func createTempFile(name string) (string, []byte, error) {
fileData := []byte{0x20}
fname := filepath.Join(tmpDir, name)
if err := ioutil.WriteFile(fname, []byte{0x20}, 0644); err != nil {
return "", err
if err := ioutil.WriteFile(fname, fileData, 0644); err != nil {
return "", nil, err
}
return fname, nil
return fname, fileData, nil
}

func createTempFileContent(name, value string) (string, error) {
Expand Down Expand Up @@ -247,6 +248,7 @@ var _ = Describe("Config Operations", func() {
gomega.Expect(CNI.Plugin).To(gomega.Equal("ovn-k8s-cni-overlay"))
gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(""))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(""))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal([]byte{}))
gomega.Expect(Kubernetes.Token).To(gomega.Equal(""))
gomega.Expect(Kubernetes.APIServer).To(gomega.Equal(DefaultAPIServer))
gomega.Expect(Kubernetes.RawServiceCIDRs).To(gomega.Equal("172.16.1.0/24"))
Expand Down Expand Up @@ -289,7 +291,7 @@ var _ = Describe("Config Operations", func() {
Output: "asadfasdfasrw3atr3r3rf33fasdaa3233",
})
// k8s-ca-certificate
fname, err := createTempFile("ca.crt")
fname, fdata, err := createTempFile("ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=15 --if-exists get Open_vSwitch . external_ids:k8s-ca-certificate",
Expand All @@ -313,6 +315,7 @@ var _ = Describe("Config Operations", func() {

gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://somewhere.com:8081"))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(fname))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(fdata))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("asadfasdfasrw3atr3r3rf33fasdaa3233"))

gomega.Expect(OvnNorth.Scheme).To(gomega.Equal(OvnDBSchemeTCP))
Expand Down Expand Up @@ -351,7 +354,7 @@ var _ = Describe("Config Operations", func() {
Output: "asadfasdfasrw3atr3r3rf33fasdaa3233",
})
// k8s-ca-certificate
fname, err := createTempFile("kube-cacert.pem")
fname, fdata, err := createTempFile("kube-cacert.pem")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=15 --if-exists get Open_vSwitch . external_ids:k8s-ca-certificate",
Expand Down Expand Up @@ -379,6 +382,7 @@ var _ = Describe("Config Operations", func() {

gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://somewhere.com:8081"))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(fname))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(fdata))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("asadfasdfasrw3atr3r3rf33fasdaa3233"))

gomega.Expect(OvnNorth.Scheme).To(gomega.Equal(OvnDBSchemeTCP))
Expand All @@ -403,9 +407,9 @@ var _ = Describe("Config Operations", func() {
})

It("uses serviceaccount files", func() {
kubeCAcertFile, err := createTempFile("ca.crt")
caFile, caData, err := createTempFile("ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAcertFile)
defer os.Remove(caFile)

tokenFile, err1 := createTempFileContent("token", "TG9yZW0gaXBzdW0gZ")
gomega.Expect(err1).NotTo(gomega.HaveOccurred())
Expand All @@ -415,7 +419,8 @@ var _ = Describe("Config Operations", func() {
_, err := InitConfigSa(ctx, kexec.New(), tmpDir, nil)
gomega.Expect(err).NotTo(gomega.HaveOccurred())

gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAcertFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(caFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(caData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("TG9yZW0gaXBzdW0gZ"))

return nil
Expand All @@ -426,7 +431,7 @@ var _ = Describe("Config Operations", func() {
})

It("uses environment variables", func() {
kubeconfigEnvFile, err := createTempFile("kubeconfig.env")
kubeconfigEnvFile, _, err := createTempFile("kubeconfig.env")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeconfigEnvFile)
os.Setenv("KUBECONFIG", kubeconfigEnvFile)
Expand All @@ -438,7 +443,7 @@ var _ = Describe("Config Operations", func() {
os.Setenv("K8S_APISERVER", "https://9.2.3.4:6443")
defer os.Setenv("K8S_APISERVER", "")

kubeCAFile, err1 := createTempFile("kube-ca.crt")
kubeCAFile, kubeCAData, err1 := createTempFile("kube-ca.crt")
gomega.Expect(err1).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAFile)
os.Setenv("K8S_CACERT", kubeCAFile)
Expand All @@ -452,6 +457,7 @@ var _ = Describe("Config Operations", func() {

gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(kubeconfigEnvFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(kubeCAData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("this is the token test"))
gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://9.2.3.4:6443"))

Expand All @@ -463,11 +469,11 @@ var _ = Describe("Config Operations", func() {
})

It("overrides defaults with config file options", func() {
kubeconfigFile, err := createTempFile("kubeconfig")
kubeconfigFile, _, err := createTempFile("kubeconfig")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeconfigFile)

kubeCAFile, err := createTempFile("kube-ca.crt")
kubeCAFile, kubeCAData, err := createTempFile("kube-ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAFile)

Expand All @@ -492,6 +498,7 @@ var _ = Describe("Config Operations", func() {
gomega.Expect(CNI.Plugin).To(gomega.Equal("ovn-k8s-cni-overlay22"))
gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(kubeconfigFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(kubeCAData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("TG9yZW0gaXBzdW0gZ"))
gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://1.2.3.4:6443"))
gomega.Expect(Kubernetes.RawServiceCIDRs).To(gomega.Equal("172.18.0.0/24"))
Expand Down Expand Up @@ -533,11 +540,11 @@ var _ = Describe("Config Operations", func() {
})

It("overrides config file and defaults with CLI options", func() {
kubeconfigFile, err := createTempFile("kubeconfig")
kubeconfigFile, _, err := createTempFile("kubeconfig")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeconfigFile)

kubeCAFile, err := createTempFile("kube-ca.crt")
kubeCAFile, kubeCAData, err := createTempFile("kube-ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAFile)

Expand All @@ -559,6 +566,7 @@ var _ = Describe("Config Operations", func() {
gomega.Expect(CNI.Plugin).To(gomega.Equal("a-plugin"))
gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(kubeconfigFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(kubeCAData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("asdfasdfasdfasfd"))
gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://4.4.3.2:8080"))
gomega.Expect(Kubernetes.RawServiceCIDRs).To(gomega.Equal("172.15.0.0/24"))
Expand Down Expand Up @@ -847,11 +855,11 @@ mode=shared
gomega.Expect(err).NotTo(gomega.HaveOccurred())
})
It("overrides config file and defaults with CLI options (multi-master)", func() {
kubeconfigFile, err := createTempFile("kubeconfig")
kubeconfigFile, _, err := createTempFile("kubeconfig")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeconfigFile)

kubeCAFile, err := createTempFile("kube-ca.crt")
kubeCAFile, kubeCAData, err := createTempFile("kube-ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAFile)

Expand All @@ -875,6 +883,7 @@ mode=shared
gomega.Expect(CNI.Plugin).To(gomega.Equal("a-plugin"))
gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(kubeconfigFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(kubeCAData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("asdfasdfasdfasfd"))
gomega.Expect(Kubernetes.APIServer).To(gomega.Equal("https://4.4.3.2:8080"))
gomega.Expect(Kubernetes.RawNoHostSubnetNodes).To(gomega.Equal("label=another-test-label"))
Expand Down Expand Up @@ -931,11 +940,11 @@ mode=shared
})

It("does not override config file settings with default cli options", func() {
kubeconfigFile, err := createTempFile("kubeconfig")
kubeconfigFile, _, err := createTempFile("kubeconfig")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeconfigFile)

kubeCAFile, err := createTempFile("kube-ca.crt")
kubeCAFile, kubeCAData, err := createTempFile("kube-ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
defer os.Remove(kubeCAFile)

Expand All @@ -960,6 +969,7 @@ mode=shared
gomega.Expect(CNI.Plugin).To(gomega.Equal("ovn-k8s-cni-overlay22"))
gomega.Expect(Kubernetes.Kubeconfig).To(gomega.Equal(kubeconfigFile))
gomega.Expect(Kubernetes.CACert).To(gomega.Equal(kubeCAFile))
gomega.Expect(Kubernetes.CAData).To(gomega.Equal(kubeCAData))
gomega.Expect(Kubernetes.Token).To(gomega.Equal("TG9yZW0gaXBzdW0gZ"))
gomega.Expect(Kubernetes.RawServiceCIDRs).To(gomega.Equal("172.18.0.0/24"))

Expand Down Expand Up @@ -1124,9 +1134,9 @@ mode=shared

BeforeEach(func() {
var err error
certFile, err = createTempFile("cert.crt")
certFile, _, err = createTempFile("cert.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
keyFile, err = createTempFile("priv.key")
keyFile, _, err = createTempFile("priv.key")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
caFile = filepath.Join(tmpDir, "ca.crt")
})
Expand Down Expand Up @@ -1282,7 +1292,7 @@ mode=shared
Describe("Kubernetes config options", func() {
Context("returns an error when the", func() {
generateTestsSimple("CA cert does not exist",
"kubernetes CA certificate file \"/foo/bar/baz.cert\" not found",
"open /foo/bar/baz.cert: no such file or directory",
"-k8s-apiserver=https://localhost:8443", "-k8s-cacert=/foo/bar/baz.cert")

generateTestsSimple("apiserver URL scheme is invalid",
Expand All @@ -1304,11 +1314,11 @@ mode=shared

BeforeEach(func() {
var err error
certFile, err = createTempFile("cert.crt")
certFile, _, err = createTempFile("cert.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
keyFile, err = createTempFile("priv.key")
keyFile, _, err = createTempFile("priv.key")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
caFile, err = createTempFile("ca.crt")
caFile, _, err = createTempFile("ca.crt")
gomega.Expect(err).NotTo(gomega.HaveOccurred())
})

Expand Down
20 changes: 9 additions & 11 deletions go-controller/pkg/util/kube.go
View file
Expand Up @@ -52,7 +52,8 @@ func adjustNodeName() string {
}

// newKubernetesRestConfig create a Kubernetes rest config from either a kubeconfig,
// TLS properties, or an apiserver URL
// TLS properties, or an apiserver URL. If the CA certificate data is passed in the
// CAData in the KubernetesConfig, the CACert path is ignored.
func newKubernetesRestConfig(conf *config.KubernetesConfig) (*rest.Config, error) {
var kconfig *rest.Config
var err error
Expand All @@ -61,19 +62,16 @@ func newKubernetesRestConfig(conf *config.KubernetesConfig) (*rest.Config, error
// uses the current context in kubeconfig
kconfig, err = clientcmd.BuildConfigFromFlags("", conf.Kubeconfig)
} else if strings.HasPrefix(conf.APIServer, "https") {
// TODO: Looks like the check conf.APIServer is redundant and can be removed
if conf.APIServer == "" || conf.Token == "" {
if conf.Token == "" || len(conf.CAData) == 0 {
return nil, fmt.Errorf("TLS-secured apiservers require token and CA certificate")
}
kconfig = &rest.Config{
Host: conf.APIServer,
BearerToken: conf.Token,
if _, err := cert.NewPoolFromBytes(conf.CAData); err != nil {
return nil, err
}
if conf.CACert != "" {
if _, err := cert.NewPool(conf.CACert); err != nil {
return nil, err
}
kconfig.TLSClientConfig = rest.TLSClientConfig{CAFile: conf.CACert}
kconfig = &rest.Config{
Host: conf.APIServer,
BearerToken: conf.Token,
TLSClientConfig: rest.TLSClientConfig{CAData: conf.CAData},
}
} else if strings.HasPrefix(conf.APIServer, "http") {
kconfig, err = clientcmd.BuildConfigFromFlags(conf.APIServer, "")
Expand Down
31 changes: 29 additions & 2 deletions go-controller/pkg/util/kube_test.go
View file
Expand Up @@ -17,6 +17,32 @@ import (
utilpointer "k8s.io/utils/pointer"
)

// Go Daddy Class 2 CA
const validCACert string = `-----BEGIN CERTIFICATE-----
MIIEADCCAuigAwIBAgIBADANBgkqhkiG9w0BAQUFADBjMQswCQYDVQQGEwJVUzEh
MB8GA1UEChMYVGhlIEdvIERhZGR5IEdyb3VwLCBJbmMuMTEwLwYDVQQLEyhHbyBE
YWRkeSBDbGFzcyAyIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MB4XDTA0MDYyOTE3
MDYyMFoXDTM0MDYyOTE3MDYyMFowYzELMAkGA1UEBhMCVVMxITAfBgNVBAoTGFRo
ZSBHbyBEYWRkeSBHcm91cCwgSW5jLjExMC8GA1UECxMoR28gRGFkZHkgQ2xhc3Mg
MiBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTCCASAwDQYJKoZIhvcNAQEBBQADggEN
ADCCAQgCggEBAN6d1+pXGEmhW+vXX0iG6r7d/+TvZxz0ZWizV3GgXne77ZtJ6XCA
PVYYYwhv2vLM0D9/AlQiVBDYsoHUwHU9S3/Hd8M+eKsaA7Ugay9qK7HFiH7Eux6w
wdhFJ2+qN1j3hybX2C32qRe3H3I2TqYXP2WYktsqbl2i/ojgC95/5Y0V4evLOtXi
EqITLdiOr18SPaAIBQi2XKVlOARFmR6jYGB0xUGlcmIbYsUfb18aQr4CUWWoriMY
avx4A6lNf4DD+qta/KFApMoZFv6yyO9ecw3ud72a9nmYvLEHZ6IVDd2gWMZEewo+
YihfukEHU1jPEX44dMX4/7VpkI+EdOqXG68CAQOjgcAwgb0wHQYDVR0OBBYEFNLE
sNKR1EwRcbNhyz2h/t2oatTjMIGNBgNVHSMEgYUwgYKAFNLEsNKR1EwRcbNhyz2h
/t2oatTjoWekZTBjMQswCQYDVQQGEwJVUzEhMB8GA1UEChMYVGhlIEdvIERhZGR5
IEdyb3VwLCBJbmMuMTEwLwYDVQQLEyhHbyBEYWRkeSBDbGFzcyAyIENlcnRpZmlj
YXRpb24gQXV0aG9yaXR5ggEAMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQAD
ggEBADJL87LKPpH8EsahB4yOd6AzBhRckB4Y9wimPQoZ+YeAEW5p5JYXMP80kWNy
OO7MHAGjHZQopDH2esRU1/blMVgDoszOYtuURXO1v0XJJLXVggKtI3lpjbi2Tc7P
TMozI+gciKqdi0FuFskg5YmezTvacPd+mSYgFFQlq25zheabIZ0KbIIOqPjCDPoQ
HmyW74cNxA9hi63ugyuV+I6ShHI56yDqg+2DzZduCLzrTia2cyvk0/ZM/iZx4mER
dEr/VxqHD3VILs9RaRegAhJhldXRQLIQTO7ErBBDpqWeCtWVYpoNz4iCxTIM5Cuf
ReYNnyicsbkqWletNw+vHX/bvZ8=
-----END CERTIFICATE-----`

func TestNewClientset(t *testing.T) {
tests := []struct {
desc string
Expand All @@ -38,9 +64,9 @@ func TestNewClientset(t *testing.T) {
errExpected: true,
},
{
desc: "error: CACert invalid for https config",
desc: "error: CAData invalid for https config",
inpConfig: config.KubernetesConfig{
CACert: "testCert",
CAData: []byte("testCert"),
APIServer: "https",
Token: "testToken",
},
Expand All @@ -51,6 +77,7 @@ func TestNewClientset(t *testing.T) {
inpConfig: config.KubernetesConfig{
APIServer: "https",
Token: "testToken",
CAData: []byte(validCACert),
},
},
{
Expand Down

0 comments on commit 7156032

Please sign in to comment.