diff --git a/pkg/controller/apmserver/config.go b/pkg/controller/apmserver/config.go index 5cd48add42..bf781ce567 100644 --- a/pkg/controller/apmserver/config.go +++ b/pkg/controller/apmserver/config.go @@ -19,6 +19,7 @@ import ( "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/certificates" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/reconciler" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/v2/pkg/utils/k8s" ) @@ -26,8 +27,9 @@ const ( // DefaultHTTPPort is the (default) port used by ApmServer DefaultHTTPPort = 8200 - APMServerHost = "apm-server.host" - APMServerSecretToken = "apm-server.secret_token" //nolint:gosec + APMServerHost = "apm-server.host" + APMServerLegacySecretToken = "apm-server.secret_token" //nolint:gosec + APMServerSecretToken = "apm-server.auth.secret_token" //nolint:gosec APMServerSSLEnabled = "apm-server.ssl.enabled" APMServerSSLKey = "apm-server.ssl.key" @@ -40,11 +42,18 @@ func certificatesDir(associationType commonv1.AssociationType) string { return fmt.Sprintf("config/%s-certs", associationType) } +func apmServerSecretTokenKeyFor(v version.Version) string { + if v.GTE(version.MinFor(8, 0, 0)) { + return APMServerSecretToken + } + return APMServerLegacySecretToken +} + // reconcileApmServerConfig reconciles the configuration of the APM server: it first creates the configuration from the APM // specification and then reconcile the underlying secret. -func reconcileApmServerConfig(ctx context.Context, client k8s.Client, as *apmv1.ApmServer) (corev1.Secret, error) { +func reconcileApmServerConfig(ctx context.Context, client k8s.Client, as *apmv1.ApmServer, version version.Version) (corev1.Secret, error) { // Create a new configuration from the APM object spec. - cfg, err := newConfigFromSpec(ctx, client, as) + cfg, err := newConfigFromSpec(ctx, client, as, version) if err != nil { return corev1.Secret{}, err } @@ -68,10 +77,10 @@ func reconcileApmServerConfig(ctx context.Context, client k8s.Client, as *apmv1. return reconciler.ReconcileSecret(ctx, client, expectedConfigSecret, as) } -func newConfigFromSpec(ctx context.Context, c k8s.Client, as *apmv1.ApmServer) (*settings.CanonicalConfig, error) { +func newConfigFromSpec(ctx context.Context, c k8s.Client, as *apmv1.ApmServer, version version.Version) (*settings.CanonicalConfig, error) { cfg := settings.MustCanonicalConfig(map[string]interface{}{ - APMServerHost: fmt.Sprintf(":%d", DefaultHTTPPort), - APMServerSecretToken: "${SECRET_TOKEN}", + APMServerHost: fmt.Sprintf(":%d", DefaultHTTPPort), + apmServerSecretTokenKeyFor(version): "${SECRET_TOKEN}", }) esConfig, err := newElasticsearchConfigFromSpec(ctx, c, apmv1.ApmEsAssociation{ApmServer: as}) diff --git a/pkg/controller/apmserver/config_test.go b/pkg/controller/apmserver/config_test.go index c5c9830b57..71f90fed71 100644 --- a/pkg/controller/apmserver/config_test.go +++ b/pkg/controller/apmserver/config_test.go @@ -16,6 +16,7 @@ import ( apmv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/apm/v1" commonv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/common/v1" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/v2/pkg/utils/k8s" ) @@ -25,11 +26,23 @@ func TestNewConfigFromSpec(t *testing.T) { configOverrides map[string]interface{} esAssocConf *commonv1.AssociationConf kbAssocConf *commonv1.AssociationConf + version version.Version wantConf map[string]interface{} wantErr bool }{ { - name: "default config", + name: "default config", + version: version.MinFor(8, 0, 0), + wantConf: map[string]interface{}{ + "apm-server.auth.secret_token": "${SECRET_TOKEN}", + }, + }, + { + name: "default config pre 8.0", + version: version.MinFor(7, 0, 0), + wantConf: map[string]interface{}{ + "apm-server.secret_token": "${SECRET_TOKEN}", + }, }, { name: "with overridden config", @@ -39,6 +52,7 @@ func TestNewConfigFromSpec(t *testing.T) { wantConf: map[string]interface{}{ "apm-server.secret_token": "MYSECRET", }, + version: version.MinFor(7, 0, 0), }, { name: "without Elasticsearch CA cert", @@ -50,10 +64,13 @@ func TestNewConfigFromSpec(t *testing.T) { URL: "https://test-es-http.default.svc:9200", }, wantConf: map[string]interface{}{ + // version specific auth token + "apm-server.auth.secret_token": "${SECRET_TOKEN}", "output.elasticsearch.hosts": []string{"https://test-es-http.default.svc:9200"}, "output.elasticsearch.username": "elastic", "output.elasticsearch.password": "password", }, + version: version.MinFor(8, 0, 0), }, { name: "with Elasticsearch CA cert", @@ -65,11 +82,13 @@ func TestNewConfigFromSpec(t *testing.T) { URL: "https://test-es-http.default.svc:9200", }, wantConf: map[string]interface{}{ + "apm-server.auth.secret_token": "${SECRET_TOKEN}", "output.elasticsearch.hosts": []string{"https://test-es-http.default.svc:9200"}, "output.elasticsearch.username": "elastic", "output.elasticsearch.password": "password", "output.elasticsearch.ssl.certificate_authorities": []string{"config/elasticsearch-certs/ca.crt"}, }, + version: version.MinFor(8, 0, 0), }, { name: "missing auth secret", @@ -81,6 +100,7 @@ func TestNewConfigFromSpec(t *testing.T) { URL: "https://test-es-http.default.svc:9200", }, wantErr: true, + version: version.MinFor(8, 0, 0), }, { name: "Kibana and Elasticsearch configuration", @@ -110,7 +130,10 @@ func TestNewConfigFromSpec(t *testing.T) { "apm-server.kibana.username": "apm-kb-user", "apm-server.kibana.password": "password-kb-user", "apm-server.kibana.ssl.certificate_authorities": []string{"config/kibana-certs/ca.crt"}, + // version specific auth token + "apm-server.auth.secret_token": "${SECRET_TOKEN}", }, + version: version.MinFor(8, 0, 0), }, { name: "Elasticsearch fully configured and Kibana configuration without CA", @@ -139,7 +162,10 @@ func TestNewConfigFromSpec(t *testing.T) { "apm-server.kibana.host": "https://test-kb-http.default.svc:9200", "apm-server.kibana.username": "apm-kb-user", "apm-server.kibana.password": "password-kb-user", + // version specific auth token + "apm-server.auth.secret_token": "${SECRET_TOKEN}", }, + version: version.MinFor(8, 0, 0), }, } @@ -158,7 +184,7 @@ func TestNewConfigFromSpec(t *testing.T) { apmv1.NewApmEsAssociation(apmServer).SetAssociationConf(tc.esAssocConf) apmv1.NewApmKibanaAssociation(apmServer).SetAssociationConf(tc.kbAssocConf) - gotConf, err := newConfigFromSpec(context.Background(), client, apmServer) + gotConf, err := newConfigFromSpec(context.Background(), client, apmServer, tc.version) if tc.wantErr { require.Error(t, err) return @@ -198,7 +224,6 @@ func mkConf(t *testing.T, overrides map[string]interface{}) *settings.CanonicalC t.Helper() cfg, err := settings.NewCanonicalConfigFrom(map[string]interface{}{ "apm-server.host": ":8200", - "apm-server.secret_token": "${SECRET_TOKEN}", "apm-server.ssl.certificate": "/mnt/elastic-internal/http-certs/tls.crt", "apm-server.ssl.enabled": true, "apm-server.ssl.key": "/mnt/elastic-internal/http-certs/tls.key", diff --git a/pkg/controller/apmserver/controller.go b/pkg/controller/apmserver/controller.go index ea35db0df6..2889f5bd09 100644 --- a/pkg/controller/apmserver/controller.go +++ b/pkg/controller/apmserver/controller.go @@ -262,7 +262,7 @@ func (r *ReconcileApmServer) doReconcile(ctx context.Context, as *apmv1.ApmServe return results, state // will eventually retry } - state, err = r.reconcileApmServerDeployment(ctx, state, as) + state, err = r.reconcileApmServerDeployment(ctx, state, as, asVersion) if err != nil { if apierrors.IsConflict(err) { log.V(1).Info("Conflict while updating status") diff --git a/pkg/controller/apmserver/deployment.go b/pkg/controller/apmserver/deployment.go index 8f889f6a72..577c356645 100644 --- a/pkg/controller/apmserver/deployment.go +++ b/pkg/controller/apmserver/deployment.go @@ -19,14 +19,11 @@ import ( "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/deployment" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/keystore" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/tracing" + "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/v2/pkg/utils/k8s" ) -func (r *ReconcileApmServer) reconcileApmServerDeployment( - ctx context.Context, - state State, - as *apmv1.ApmServer, -) (State, error) { +func (r *ReconcileApmServer) reconcileApmServerDeployment(ctx context.Context, state State, as *apmv1.ApmServer, version version.Version) (State, error) { span, ctx := apm.StartSpan(ctx, "reconcile_deployment", tracing.SpanTypeApp) defer span.End() @@ -34,7 +31,7 @@ func (r *ReconcileApmServer) reconcileApmServerDeployment( if err != nil { return state, err } - reconciledConfigSecret, err := reconcileApmServerConfig(ctx, r.Client, as) + reconciledConfigSecret, err := reconcileApmServerConfig(ctx, r.Client, as, version) if err != nil { return state, err } diff --git a/pkg/controller/common/http/http_client.go b/pkg/controller/common/http/http_client.go index 804587c5ac..2d0746edad 100644 --- a/pkg/controller/common/http/http_client.go +++ b/pkg/controller/common/http/http_client.go @@ -100,6 +100,16 @@ func IsNotFound(err error) bool { return isHTTPError(err, http.StatusNotFound) } +// IsUnauthorized checks whether the error was an HTTP 401 error. +func IsUnauthorized(err error) bool { + return isHTTPError(err, http.StatusUnauthorized) +} + +// IsForbidden checks whether the error was an HTTP 403 error. +func IsForbidden(err error) bool { + return isHTTPError(err, http.StatusForbidden) +} + func isHTTPError(err error, statusCode int) bool { apiErr := new(APIError) if errors.As(err, &apiErr) { diff --git a/test/e2e/test/apmserver/checks_apm.go b/test/e2e/test/apmserver/checks_apm.go index 6e6442531f..e6b6dfe434 100644 --- a/test/e2e/test/apmserver/checks_apm.go +++ b/test/e2e/test/apmserver/checks_apm.go @@ -21,6 +21,7 @@ import ( apmv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/apm/v1" esv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/elasticsearch/v1" kbv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/kibana/v1" + commonhttp "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/http" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/elasticsearch/client" "github.com/elastic/cloud-on-k8s/v2/pkg/utils/k8s" @@ -30,6 +31,10 @@ import ( "github.com/elastic/cloud-on-k8s/v2/test/e2e/test/kibana" ) +const sampleEventBody = `{"metadata": { "service": {"name": "1234_service-12a3", "language": {"name": "ecmascript"}, "agent": {"version": "3.14.0", "name": "elastic-node"}}}} +{ "error": {"id": "abcdef0123456789", "timestamp": 1533827045999000,"log": {"level": "custom log level","message": "Cannot read property 'baz' of undefined"}}} +{ "metricset": { "samples": { "go.memstats.heap.sys.bytes": { "value": 61235 } }, "timestamp": 1496170422281000 }}` + type apmClusterChecks struct { apmClient *ApmClient esClient client.Client @@ -43,6 +48,7 @@ func (b Builder) CheckStackTestSteps(k *test.K8sClient) test.StepList { a.BuildApmServerClient(b.ApmServer, k), a.CheckApmServerReachable(), a.CheckApmServerVersion(b.ApmServer), + a.CheckAPMSecretTokenConfiguration(b.ApmServer, k), a.CheckAPMEventCanBeIndexedInElasticsearch(b.ApmServer, k), a.CheckRUMEventsAPI(b.RUMEnabled()), }.WithSteps(a.CheckAgentConfiguration(b.ApmServer, k)) @@ -147,10 +153,37 @@ func (c *apmClusterChecks) CheckAPMEventCanBeIndexedInElasticsearch(apm apmv1.Ap } } +func (c *apmClusterChecks) CheckAPMSecretTokenConfiguration(apm apmv1.ApmServer, k *test.K8sClient) test.Step { + return test.Step{ + Name: "APMServer should reject events with incorrect token setup", + Test: test.Eventually(func() error { + // All APM Server tests do not have an Elasticsearch reference. + if !apm.Spec.ElasticsearchRef.IsDefined() { + return nil + } + + // as above for the functioning client: fetch the latest APM Server resource from the API because we need to + // get resources that are provided by the controller apm part of the status section + var updatedApmServer apmv1.ApmServer + if err := k.Client.Get(context.Background(), k8s.ExtractNamespacedName(&apm), &updatedApmServer); err != nil { + return err + } + client, err := NewAPMServerClientWithSecretToken(updatedApmServer, k, "not-a-valid-token") + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(context.Background(), DefaultReqTimeout) + defer cancel() + _, err = client.IntakeV2Events(ctx, false, []byte(sampleEventBody)) + if !commonhttp.IsUnauthorized(err) { + return fmt.Errorf("expected error 401 but was %w", err) + } + return nil + }), + } +} + func (c *apmClusterChecks) checkEventsAPI(apm apmv1.ApmServer) error { - sampleBody := `{"metadata": { "service": {"name": "1234_service-12a3", "language": {"name": "ecmascript"}, "agent": {"version": "3.14.0", "name": "elastic-node"}}}} -{ "error": {"id": "abcdef0123456789", "timestamp": 1533827045999000,"log": {"level": "custom log level","message": "Cannot read property 'baz' of undefined"}}} -{ "metricset": { "samples": { "go.memstats.heap.sys.bytes": { "value": 61235 } }, "timestamp": 1496170422281000 }}` // before sending event, get the document count in the metric, and error index // and save, as it is used to calculate how many docs should be in the index after // the event is sent through APM Server. @@ -176,7 +209,7 @@ func (c *apmClusterChecks) checkEventsAPI(apm apmv1.ApmServer) error { ctx, cancel := context.WithTimeout(context.Background(), DefaultReqTimeout) defer cancel() - eventsErrorResponse, err := c.apmClient.IntakeV2Events(ctx, false, []byte(sampleBody)) + eventsErrorResponse, err := c.apmClient.IntakeV2Events(ctx, false, []byte(sampleEventBody)) if err != nil { return err } @@ -189,15 +222,24 @@ func (c *apmClusterChecks) checkEventsAPI(apm apmv1.ApmServer) error { return nil } +func assertHTTP403(t assert.TestingT, err error, msgAndArgs ...interface{}) bool { + if !commonhttp.IsForbidden(err) { + return assert.Fail(t, fmt.Sprintf("expected HTTP 403 but was %+v", err), msgAndArgs) + } + return true +} + func (c *apmClusterChecks) CheckRUMEventsAPI(rumEnabled bool) test.Step { sampleBody := `{"metadata":{"service":{"name":"apm-agent-js","version":"1.0.0","agent":{"name":"rum-js","version":"0.0.0"}}}} {"transaction":{"id":"611f4fa950f04631","type":"page-load","duration":643,"context":{"page":{"referer":"http://localhost:8000/test/e2e/","url":"http://localhost:8000/test/e2e/general-usecase/"}},"trace_id":"611f4fa950f04631aaaaaaaaaaaaaaaa","span_count":{"started":1}}}` should := "forbidden" - assertError := assert.NotNil + assertApplicationError := assert.NotNil + assertRequestError := assertHTTP403 if rumEnabled { should = "accepted" - assertError = assert.Nil + assertApplicationError = assert.Nil + assertRequestError = assert.NoError } //nolint:thelper return test.Step{ @@ -206,9 +248,8 @@ func (c *apmClusterChecks) CheckRUMEventsAPI(rumEnabled bool) test.Step { ctx, cancel := context.WithTimeout(context.Background(), DefaultReqTimeout) defer cancel() eventsErrorResponse, err := c.apmClient.IntakeV2Events(ctx, true, []byte(sampleBody)) - require.NoError(t, err) - - assertError(t, eventsErrorResponse) + assertRequestError(t, err) + assertApplicationError(t, eventsErrorResponse) }, } } diff --git a/test/e2e/test/apmserver/http_client.go b/test/e2e/test/apmserver/http_client.go index de45600199..3450920a08 100644 --- a/test/e2e/test/apmserver/http_client.go +++ b/test/e2e/test/apmserver/http_client.go @@ -20,6 +20,7 @@ import ( apmv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/apm/v1" "github.com/elastic/cloud-on-k8s/v2/pkg/controller/apmserver" + commonhttp "github.com/elastic/cloud-on-k8s/v2/pkg/controller/common/http" "github.com/elastic/cloud-on-k8s/v2/pkg/utils/stringsutil" "github.com/elastic/cloud-on-k8s/v2/test/e2e/test" ) @@ -45,7 +46,15 @@ func NewApmServerClient(as apmv1.ApmServer, k *test.K8sClient) (*ApmClient, erro if err := k.Client.Get(context.Background(), secretTokenNamespacedName, &secretTokenSecret); err != nil { return nil, err } + secretToken, ok := secretTokenSecret.Data[apmserver.SecretTokenKey] + if !ok { + return nil, fmt.Errorf("secret token not found in secret: %s", as.Status.SecretTokenSecretName) + } + + return NewAPMServerClientWithSecretToken(as, k, string(secretToken)) +} +func NewAPMServerClientWithSecretToken(as apmv1.ApmServer, k *test.K8sClient, secretToken string) (*ApmClient, error) { scheme := "http" var caCerts []*x509.Certificate if as.Spec.HTTP.TLS.Enabled() { @@ -63,11 +72,6 @@ func NewApmServerClient(as apmv1.ApmServer, k *test.K8sClient) (*ApmClient, erro client := test.NewHTTPClient(caCerts) - secretToken, ok := secretTokenSecret.Data[apmserver.SecretTokenKey] - if !ok { - return nil, fmt.Errorf("secret token not found in secret: %s", as.Status.SecretTokenSecretName) - } - return &ApmClient{ client: client, endpoint: inClusterURL, @@ -220,7 +224,7 @@ func (c *ApmClient) IntakeV2Events(ctx context.Context, rum bool, payload []byte return nil, err } - return &eventsErrorResponse, err + return &eventsErrorResponse, commonhttp.MaybeAPIError(resp) } // AgentConfig describes an agent configuration