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

Fix Stack Monitoring with custom certificate without CA #5310

Merged
merged 11 commits into from Feb 2, 2022
Merged

Fix Stack Monitoring with custom certificate without CA #5310

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
db32512
Fix Stack Monitoring without CA
thbkrkr Jan 31, 2022
a4ab1ac
lint
thbkrkr Jan 31, 2022
a349bba
Switch to table driven tests
thbkrkr Feb 1, 2022
2e19d17
s/isCA/hasCA/
thbkrkr Feb 1, 2022
84eee1b
s/HasPublicCA/PublicCertsHasCACert/
thbkrkr Feb 1, 2022
4a1df5e
Apply review's input
thbkrkr Feb 1, 2022
2fe886a
Fix unit test name
thbkrkr Feb 1, 2022
9ad998b
Reordering
thbkrkr Feb 1, 2022
3911da6
Move verification mode in the templates
thbkrkr Feb 1, 2022
c0994ab
Do not resolve hasCA by getting HTTP public certs secret if no TLS
thbkrkr Feb 1, 2022
e242ab0
Update unit test
thbkrkr Feb 1, 2022
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
21 changes: 21 additions & 0 deletions pkg/controller/common/certificates/ca.go
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@
package certificates

import (
"context"
"crypto"
cryptorand "crypto/rand"
"crypto/rsa"
Expand All @@ -14,6 +15,11 @@ import (
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/elastic/cloud-on-k8s/pkg/controller/common/name"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
)

var (
Expand Down Expand Up @@ -124,3 +130,18 @@ func (c *CA) CreateCertificate(

return certData, err
}

// HasPublicCA returns true if an Elastic resource has a CA (ca.crt) in its public HTTP certs secret given its namer,
// namespace and name.
func HasPublicCA(client k8s.Client, namer name.Namer, namespace string, name string) (bool, error) {
thbkrkr marked this conversation as resolved.
Show resolved Hide resolved
certsNsn := types.NamespacedName{
Name: PublicCertsSecretName(namer, name),
Namespace: namespace,
}
var certsSecret corev1.Secret
if err := client.Get(context.Background(), certsNsn, &certsSecret); err != nil {
return false, err
}
_, ok := certsSecret.Data[CAFileName]
return ok, nil
}
38 changes: 38 additions & 0 deletions pkg/controller/common/certificates/ca_test.go
View file
Open in desktop
Expand Up @@ -15,6 +15,11 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
)

func init() {
Expand Down Expand Up @@ -85,3 +90,36 @@ func TestNewSelfSignedCA(t *testing.T) {
require.Equal(t, testRSAPrivateKey, ca.PrivateKey)
require.True(t, ca.Cert.NotBefore.Before(time.Now().Add(2*time.Hour)))
}

func Test_HasPublicCA(t *testing.T) {
certsWithCA := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
Name: "c1-es-http-certs-public",
},
Data: map[string][]byte{
CAFileName: []byte("..."),
CertFileName: []byte("..."),
},
}
hasCA, err := HasPublicCA(k8s.NewFakeClient(&certsWithCA), esv1.ESNamer, "ns", "c1")
assert.NoError(t, err)
assert.True(t, hasCa)

certsWithoutCA := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
Name: "c2-es-http-certs-public",
},
Data: map[string][]byte{
CertFileName: []byte("..."),
},
}
hasCA, err = HasPublicCA(k8s.NewFakeClient(&certsWithoutCA), esv1.ESNamer, "ns", "c2")
assert.NoError(t, err)
assert.False(t, hasCA)

// no certs secret
_, err = HasPublicCA(k8s.NewFakeClient(), esv1.ESNamer, "ns", "c3")
assert.Error(t, err)
thbkrkr marked this conversation as resolved.
Show resolved Hide resolved
}
12 changes: 6 additions & 6 deletions pkg/controller/common/stackmon/config.go
View file
Open in desktop
Expand Up @@ -159,8 +159,8 @@ type inputConfigData struct {
URL string
Username string
Password string
IsSSL bool
SSLPath string
IsCA bool
thbkrkr marked this conversation as resolved.
Show resolved Hide resolved
CAPath string
SSLMode string
}

Expand All @@ -173,7 +173,7 @@ func buildMetricbeatBaseConfig(
esNsn types.NamespacedName,
namer name.Namer,
url string,
isTLS bool,
isCA bool,
configTemplate string,
) (string, volume.VolumeLike, error) {
password, err := user.GetMonitoringUserPassword(client, esNsn)
Expand All @@ -185,18 +185,18 @@ func buildMetricbeatBaseConfig(
URL: url,
Username: user.MonitoringUserName,
Password: password,
IsSSL: isTLS,
IsCA: isCA,
}

var caVolume volume.VolumeLike
if configData.IsSSL {
if configData.IsCA {
caVolume = volume.NewSecretVolumeWithMountPath(
certificates.PublicCertsSecretName(namer, nsn.Name),
fmt.Sprintf("%s-local-ca", string(associationType)),
fmt.Sprintf("/mnt/elastic-internal/%s/%s/%s/certs", string(associationType), nsn.Namespace, nsn.Name),
)

configData.SSLPath = filepath.Join(caVolume.VolumeMount().MountPath, certificates.CAFileName)
configData.CAPath = filepath.Join(caVolume.VolumeMount().MountPath, certificates.CAFileName)
configData.SSLMode = "certificate"
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/controller/common/stackmon/config_test.go
View file
Open in desktop
Expand Up @@ -19,12 +19,12 @@ import (
func TestBuildMetricbeatBaseConfig(t *testing.T) {
tests := []struct {
name string
isTLS bool
isCA bool
baseConfig string
}{
{
name: "with tls",
isTLS: true,
name: "with tls",
isCA: true,
baseConfig: `
hosts: ["scheme://localhost:1234"]
username: elastic-internal-monitoring
Expand All @@ -33,8 +33,8 @@ func TestBuildMetricbeatBaseConfig(t *testing.T) {
ssl.verification_mode: "certificate"`,
},
{
name: "without tls",
isTLS: false,
name: "without CA",
isCA: false,
baseConfig: `
hosts: ["scheme://localhost:1234"]
username: elastic-internal-monitoring
Expand All @@ -46,8 +46,8 @@ func TestBuildMetricbeatBaseConfig(t *testing.T) {
hosts: ["{{ .URL }}"]
username: {{ .Username }}
password: {{ .Password }}
{{- if .IsSSL }}
ssl.certificate_authorities: ["{{ .SSLPath }}"]
{{- if .IsCA }}
ssl.certificate_authorities: ["{{ .CAPath }}"]
ssl.verification_mode: "{{ .SSLMode }}"
{{- end }}`
sampleURL := "scheme://localhost:1234"
Expand All @@ -66,7 +66,7 @@ func TestBuildMetricbeatBaseConfig(t *testing.T) {
types.NamespacedName{Namespace: "namespace", Name: "name"},
name.NewNamer("xx"),
sampleURL,
tc.isTLS,
tc.isCA,
baseConfigTemplate,
)
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/common/stackmon/sidecar.go
View file
Open in desktop
Expand Up @@ -28,7 +28,7 @@ func NewMetricBeatSidecar(
baseConfigTemplate string,
namer name.Namer,
url string,
isTLS bool,
isCA bool,
) (BeatSidecar, error) {
baseConfig, sourceCaVolume, err := buildMetricbeatBaseConfig(
client,
Expand All @@ -37,7 +37,7 @@ func NewMetricBeatSidecar(
esNsn,
namer,
url,
isTLS,
isCA,
baseConfigTemplate,
)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/elasticsearch/stackmon/metricbeat.tpl.yml
View file
Open in desktop
Expand Up @@ -17,8 +17,8 @@ metricbeat.modules:
hosts: ["{{ .URL }}"]
username: {{ .Username }}
password: {{ .Password }}
{{- if .IsSSL }}
ssl.certificate_authorities: ["{{ .SSLPath }}"]
{{- if .IsCA }}
ssl.certificate_authorities: ["{{ .CAPath }}"]
ssl.verification_mode: "{{ .SSLMode }}"
{{- end }}

Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/elasticsearch/stackmon/sidecar.go
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ import (

commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/defaults"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/stackmon"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/stackmon/monitoring"
Expand All @@ -26,6 +27,10 @@ const (
)

func Metricbeat(client k8s.Client, es esv1.Elasticsearch) (stackmon.BeatSidecar, error) {
isCA, err := certificates.HasPublicCA(client, esv1.ESNamer, es.Namespace, es.Name)
if err != nil {
return stackmon.BeatSidecar{}, err
}
metricbeat, err := stackmon.NewMetricBeatSidecar(
client,
commonv1.KbMonitoringAssociationType,
Expand All @@ -35,7 +40,7 @@ func Metricbeat(client k8s.Client, es esv1.Elasticsearch) (stackmon.BeatSidecar,
metricbeatConfigTemplate,
esv1.ESNamer,
fmt.Sprintf("%s://localhost:%d", es.Spec.HTTP.Protocol(), network.HTTPPort),
es.Spec.HTTP.TLS.Enabled(),
isCA,
)
if err != nil {
return stackmon.BeatSidecar{}, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/kibana/stackmon/metricbeat.tpl.yml
View file
Open in desktop
Expand Up @@ -9,8 +9,8 @@ metricbeat.modules:
hosts: ["{{ .URL }}"]
username: {{ .Username }}
password: {{ .Password }}
{{- if .IsSSL }}
ssl.certificate_authorities: ["{{ .SSLPath }}"]
{{- if .IsCA }}
ssl.certificate_authorities: ["{{ .CAPath }}"]
ssl.verification_mode: "{{ .SSLMode }}"
thbkrkr marked this conversation as resolved.
Show resolved Hide resolved
{{- end }}

Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/kibana/stackmon/sidecar.go
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ import (

commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
kbv1 "github.com/elastic/cloud-on-k8s/pkg/apis/kibana/v1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/defaults"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/stackmon"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/stackmon/monitoring"
Expand Down Expand Up @@ -40,6 +41,10 @@ func Metricbeat(client k8s.Client, kb kbv1.Kibana) (stackmon.BeatSidecar, error)
associatedEsNsn.Namespace = kb.Namespace
}

isCA, err := certificates.HasPublicCA(client, kbv1.KBNamer, kb.Namespace, kb.Name)
if err != nil {
return stackmon.BeatSidecar{}, err
}
metricbeat, err := stackmon.NewMetricBeatSidecar(
client,
commonv1.KbMonitoringAssociationType,
Expand All @@ -49,7 +54,7 @@ func Metricbeat(client k8s.Client, kb kbv1.Kibana) (stackmon.BeatSidecar, error)
metricbeatConfigTemplate,
kbv1.KBNamer,
fmt.Sprintf("%s://localhost:%d", kb.Spec.HTTP.Protocol(), network.HTTPPort),
kb.Spec.HTTP.TLS.Enabled(),
isCA,
)
if err != nil {
return stackmon.BeatSidecar{}, err
Expand Down
14 changes: 12 additions & 2 deletions pkg/controller/kibana/stackmon/sidecar_test.go
View file
Open in desktop
Expand Up @@ -48,9 +48,19 @@ func TestWithMonitoring(t *testing.T) {
}
fakeEsHTTPCertSecret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "sample-es-http-certs-public", Namespace: "aerospace"},
Data: map[string][]byte{"ca.crt": []byte("7H1515N074r341C3r71F1C473")},
Data: map[string][]byte{
"tls.crt": []byte("7H1515N074r341C3r71F1C473"),
"ca.crt": []byte("7H1515N074r341C3r71F1C473"),
},
}
fakeKbHTTPCertSecret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "sample-kb-http-certs-public", Namespace: "aerospace"},
Data: map[string][]byte{
"tls.crt": []byte("7H1515N074r341C3r71F1C473"),
"ca.crt": []byte("7H1515N074r341C3r71F1C473"),
},
}
fakeClient := k8s.NewFakeClient(&fakeElasticUserSecret, &fakeMetricsBeatUserSecret, &fakeLogsBeatUserSecret, &fakeEsHTTPCertSecret)
fakeClient := k8s.NewFakeClient(&fakeElasticUserSecret, &fakeMetricsBeatUserSecret, &fakeLogsBeatUserSecret, &fakeEsHTTPCertSecret, &fakeKbHTTPCertSecret)

monitoringAssocConf := commonv1.AssociationConf{
AuthSecretName: "sample-observability-monitoring-beat-es-mon-user",
Expand Down