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.

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

Root CA bundle #1316

Merged
merged 11 commits into from
Apr 30, 2021
Merged
9 changes: 7 additions & 2 deletions docs/api_reference/v1beta1.en.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
+++
title = "v1beta1 API Reference"
date = 2021-01-17T17:46:19+01:00
date = 2021-04-19T12:12:05+03:00
weight = 11
+++
## v1beta1
Expand Down Expand Up @@ -269,6 +269,7 @@ Encryption Providers feature flag
| Field | Description | Scheme | Required |
| ----- | ----------- | ------ | -------- |
| enable | Enable | bool | true |
| customEncryptionConfiguration | CustomEncryptionConfiguration | string | true |

[Back to Group](#v1beta1)

Expand All @@ -289,7 +290,7 @@ Features controls what features will be enabled on the cluster
| Field | Description | Scheme | Required |
| ----- | ----------- | ------ | -------- |
| podNodeSelector | PodNodeSelector | *[PodNodeSelector](#podnodeselector) | false |
| podPresets | PodPresets | *[PodPresets](#podpresets) | false |
| podPresets | PodPresets Deprecated: will be removed once Kubernetes 1.19 reaches EOL | *[PodPresets](#podpresets) | false |
| podSecurityPolicy | PodSecurityPolicy | *[PodSecurityPolicy](#podsecuritypolicy) | false |
| staticAuditLog | StaticAuditLog | *[StaticAuditLog](#staticauditlog) | false |
| dynamicAuditLog | DynamicAuditLog | *[DynamicAuditLog](#dynamicauditlog) | false |
Expand Down Expand Up @@ -367,6 +368,7 @@ KubeOneCluster is KubeOne Cluster API Schema
| staticWorkers | StaticWorkers describes the worker nodes that are managed by KubeOne/kubeadm. | [StaticWorkersConfig](#staticworkersconfig) | false |
| dynamicWorkers | DynamicWorkers describes the worker nodes that are managed by Kubermatic machine-controller/Cluster-API. | [][DynamicWorkerConfig](#dynamicworkerconfig) | false |
| machineController | MachineController configures the Kubermatic machine-controller component. | *[MachineControllerConfig](#machinecontrollerconfig) | false |
| caBundle | CABundle PEM encoded global CA | string | false |
| features | Features enables and configures additional cluster features. | [Features](#features) | false |
| addons | Addons are used to deploy additional manifests. | *[Addons](#addons) | false |
| systemPackages | SystemPackages configure kubeone behaviour regarding OS packages. | *[SystemPackages](#systempackages) | false |
Expand Down Expand Up @@ -475,6 +477,9 @@ PodNodeSelectorConfig config
### PodPresets

PodPresets feature flag
The PodPresets feature has been removed in Kubernetes 1.20.
This feature is deprecated and will be removed from the API once
Kubernetes 1.19 reaches EOL.

| Field | Description | Scheme | Required |
| ----- | ----------- | ------ | -------- |
Expand Down
18 changes: 18 additions & 0 deletions pkg/addons/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"k8c.io/kubeone/pkg/certificate/cabundle"
"k8c.io/kubeone/pkg/state"

corev1 "k8s.io/api/core/v1"
metav1unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
kyaml "k8s.io/apimachinery/pkg/util/yaml"
Expand Down Expand Up @@ -196,12 +198,28 @@ func combineManifests(manifests []*bytes.Buffer) *bytes.Buffer {

func txtFuncMap(overwriteRegistry string) template.FuncMap {
funcs := sprig.TxtFuncMap()

funcs["Registry"] = func(registry string) string {
if overwriteRegistry != "" {
return overwriteRegistry
}
return registry
}

funcs["caBundleEnvVar"] = func() (string, error) {
buf, err := yaml.Marshal([]corev1.EnvVar{cabundle.EnvVar()})
return string(buf), err
}

funcs["caBundleVolume"] = func() (string, error) {
buf, err := yaml.Marshal([]corev1.Volume{cabundle.Volume()})
return string(buf), err
}

funcs["caBundleVolumeMount"] = func() (string, error) {
buf, err := yaml.Marshal([]corev1.VolumeMount{cabundle.VolumeMount()})
return string(buf), err
}

return funcs
}
28 changes: 28 additions & 0 deletions pkg/addons/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"io/ioutil"
"os"
"path"
"strings"
"testing"
"text/template"

kubeoneapi "k8c.io/kubeone/pkg/apis/kubeone"
)
Expand Down Expand Up @@ -252,3 +254,29 @@ func TestImageRegistryParsing(t *testing.T) {
})
}
}

func TestCABundleFuncs(t *testing.T) {
tests := []string{
"caBundleEnvVar",
"caBundleVolume",
"caBundleVolumeMount",
}

for _, tt := range tests {
tt := tt
t.Run(tt, func(t *testing.T) {
tpl, err := template.New("addons-base").Funcs(txtFuncMap("")).Parse(fmt.Sprintf(`{{ %s }}`, tt))

if err != nil {
t.Errorf("failed to parse template: %v", err)
t.FailNow()
}

var out strings.Builder
if err := tpl.Execute(&out, nil); err != nil {
t.Errorf("failed to parse template: %v", err)
}
t.Logf("\n%s", out.String())
})
}
}
2 changes: 2 additions & 0 deletions pkg/apis/kubeone/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type KubeOneCluster struct {
DynamicWorkers []DynamicWorkerConfig `json:"dynamicWorkers,omitempty"`
// MachineController configures the Kubermatic machine-controller component.
MachineController *MachineControllerConfig `json:"machineController,omitempty"`
// CABundle PEM encoded global CA
CABundle string `json:"caBundle,omitempty"`
Comment on lines +54 to +55
Copy link
Member

Choose a reason for hiding this comment

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

I'd say that this should be a path to the bundle on local FS instead of the bundle itself. The bundle can be huge (the Mozilla one is 3k+ lines).

Copy link
Member Author

@kron4eg kron4eg Apr 23, 2021

Choose a reason for hiding this comment

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

While yes, the original public CA bundle is long, this field should only be used in case when you have your very own custom CA bundle, which most likely will not be that long.

Copy link
Member

Choose a reason for hiding this comment

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

I still prefer if we would reference a file instead of embedding it in the KubeOne config manifest. We can't know for sure how long is the CA bundle. Theoretically, it can range from several dozens of lines to several thousand. From the user experience side, I believe referencing the file is better because the manifest is smaller and easier to maintain.

Additionally, we use this pattern in other places as well, such as:

  1. addons
  2. PodNodeSelector and StaticAuditLog features

Copy link
Member Author

Choose a reason for hiding this comment

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

Then let's build on top of this PR as it's already quite big. My proposal would be to retrofit the "special content" linking.
I.e. let's have file:///path/to/file in .CABundle and in case if it starts from file:// that will be interpreted as reference to the file, instead of actual content.

Copy link
Member

Choose a reason for hiding this comment

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

This sounds like a good plan. Can you create a follow-up issue for this?

// Features enables and configures additional cluster features.
Features Features `json:"features,omitempty"`
// Addons are used to deploy additional manifests.
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kubeone/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/kubeone/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type KubeOneCluster struct {
DynamicWorkers []DynamicWorkerConfig `json:"dynamicWorkers,omitempty"`
// MachineController configures the Kubermatic machine-controller component.
MachineController *MachineControllerConfig `json:"machineController,omitempty"`
// CABundle PEM encoded global CA
CABundle string `json:"caBundle,omitempty"`
// Features enables and configures additional cluster features.
Features Features `json:"features,omitempty"`
// Addons are used to deploy additional manifests.
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kubeone/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions pkg/apis/kubeone/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package validation

import (
"bytes"
"crypto/x509"
"net"
"reflect"
"strings"
Expand Down Expand Up @@ -50,6 +52,7 @@ func ValidateKubeOneCluster(c kubeone.KubeOneCluster) field.ErrorList {
"machine-controller deployment is disabled, but the configuration still contains dynamic workers"))
}

allErrs = append(allErrs, ValidateCABundle(c.CABundle, field.NewPath("caBundle"))...)
allErrs = append(allErrs, ValidateFeatures(c.Features, c.Versions, field.NewPath("features"))...)
allErrs = append(allErrs, ValidateAddons(c.Addons, field.NewPath("addons"))...)
allErrs = append(allErrs, ValidateRegistryConfiguration(c.RegistryConfiguration, field.NewPath("registryConfiguration"))...)
Expand Down Expand Up @@ -293,6 +296,22 @@ func ValidateDynamicWorkerConfig(workerset []kubeone.DynamicWorkerConfig, fldPat
return allErrs
}

func ValidateCABundle(caBundle string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

caPEM := bytes.TrimSpace([]byte(caBundle))
if len(caPEM) == 0 {
return allErrs
}

pool := x509.NewCertPool()
if ok := pool.AppendCertsFromPEM(caPEM); !ok {
allErrs = append(allErrs, field.Invalid(fldPath, "", "can't parse caBundle"))
}

return allErrs
}

// ValidateFeatures validates the Features structure
func ValidateFeatures(f kubeone.Features, versions kubeone.VersionConfig, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down
68 changes: 68 additions & 0 deletions pkg/apis/kubeone/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package validation
import (
"testing"

"github.com/MakeNowJust/heredoc/v2"

"k8c.io/kubeone/pkg/apis/kubeone"

"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -999,6 +1001,72 @@ func TestValidateDynamicWorkerConfig(t *testing.T) {
}
}

func TestValidateCABundle(t *testing.T) {
tests := []struct {
name string
caBundle string
expectedError bool
}{
{
name: "empty",
caBundle: "",
expectedError: false,
},
{
name: "correct",
caBundle: heredoc.Doc(`
## some comments

GlobalSign Root CA
==================
-----BEGIN CERTIFICATE-----
MIIDdTCCAl2gAwIBAgILBAAAAAABFUtaw5QwDQYJKoZIhvcNAQEFBQAwVzELMAkGA1UEBhMCQkUx
GTAXBgNVBAoTEEdsb2JhbFNpZ24gbnYtc2ExEDAOBgNVBAsTB1Jvb3QgQ0ExGzAZBgNVBAMTEkds
b2JhbFNpZ24gUm9vdCBDQTAeFw05ODA5MDExMjAwMDBaFw0yODAxMjgxMjAwMDBaMFcxCzAJBgNV
BAYTAkJFMRkwFwYDVQQKExBHbG9iYWxTaWduIG52LXNhMRAwDgYDVQQLEwdSb290IENBMRswGQYD
VQQDExJHbG9iYWxTaWduIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDa
DuaZjc6j40+Kfvvxi4Mla+pIH/EqsLmVEQS98GPR4mdmzxzdzxtIK+6NiY6arymAZavpxy0Sy6sc
THAHoT0KMM0VjU/43dSMUBUc71DuxC73/OlS8pF94G3VNTCOXkNz8kHp1Wrjsok6Vjk4bwY8iGlb
Kk3Fp1S4bInMm/k8yuX9ifUSPJJ4ltbcdG6TRGHRjcdGsnUOhugZitVtbNV4FpWi6cgKOOvyJBNP
c1STE4U6G7weNLWLBYy5d4ux2x8gkasJU26Qzns3dLlwR5EiUWMWea6xrkEmCMgZK9FGqkjWZCrX
gzT/LCrBbBlDSgeF59N89iFo7+ryUp9/k5DPAgMBAAGjQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNV
HRMBAf8EBTADAQH/MB0GA1UdDgQWBBRge2YaRQ2XyolQL30EzTSo//z9SzANBgkqhkiG9w0BAQUF
AAOCAQEA1nPnfE920I2/7LqivjTFKDK1fPxsnCwrvQmeU79rXqoRSLblCKOzyj1hTdNGCbM+w6Dj
Y1Ub8rrvrTnhQ7k4o+YviiY776BQVvnGCv04zcQLcFGUl5gE38NflNUVyRRBnMRddWQVDf9VMOyG
j/8N7yy5Y0b2qvzfvGn9LhJIZJrglfCm7ymPAbEVtQwdpf5pLGkkeB6zpxxxYu7KyJesF12KwvhH
hm4qxFYxldBniYUr+WymXUadDKqC5JlR3XC321Y9YeRq4VzW9v493kHMB65jUr9TU/Qr6cf9tveC
X4XSQRjbgbMEHMUfpIBvFSDJ3gyICh3WZlXi/EjJKSZp4A==
-----END CERTIFICATE-----
`),
expectedError: false,
},
{
name: "no certs but with comments",
caBundle: heredoc.Doc(`
# leading comment
## additional comment
`),
expectedError: true,
},
{
name: "incorrect",
caBundle: "garbadge",
expectedError: true,
},
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
errs := ValidateCABundle(tc.caBundle, field.NewPath("caBundle"))
if (len(errs) == 0) == tc.expectedError {
t.Logf("failed value:\n%q", tc.caBundle)
t.Errorf("test case failed: expected %v, but got %v", tc.expectedError, (len(errs) != 0))
}
})
}
}

func TestValidateFeatures(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion pkg/archive/targz.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (tgz tarGzip) Add(file string, content string) error {
return nil
}

func (tgz tarGzip) Close() {
func (tgz *tarGzip) Close() {
if tgz.arch != nil {
tgz.arch.Close()
tgz.arch = nil
Expand Down
Loading