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

remove flags for policy defaulting #30256

Closed
wants to merge 14 commits into from
1 change: 1 addition & 0 deletions x-pack/elastic-agent/CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,4 @@
- Discover changes in Kubernetes nodes metadata as soon as they happen. {pull}23139[23139]
- Add results of inspect output command into archive produced by diagnostics collect. {pull}29902[29902]
- Add support for loading input configuration from external configuration files in standalone mode. You can load inputs from YAML configuration files under the folder `{path.config}/inputs.d`. {pull}30087[30087]
- Changed the default policy selection logic. When an agent is unmanaged and no policy id/name has been specified upon enroll, the agent will select a random unmanaged policy. {issue}29774[29774] {pull}30256[30256]
4 changes: 4 additions & 0 deletions x-pack/elastic-agent/docs/fleet-server-bootstrap.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ paths based on the operating system then starts the Elastic Agent service. The
When the `container` command is executed it first copies the `data/downloads` directory
into a state path (`STATE_PATH`) then it executes the `enroll` command.

When using the `container` command, a policy ID for the agent should be supplied via the
`FLEET_SERVER_POLICY` environment variable. If this value is omitted, the agent will attempt
to use the deafult policy ID, which can be configured via `DEFALUT_FLEET_SERVER_POLICY`
lykkin marked this conversation as resolved.
Show resolved Hide resolved

==== Enroll Command

This is where all the actual work of bootstrapping is performed.
Expand Down
42 changes: 32 additions & 10 deletions x-pack/elastic-agent/pkg/agent/cmd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ The following actions are possible and grouped based on the actions.

KIBANA_FLEET_HOST - kibana host to enable create enrollment token on [$KIBANA_HOST]
FLEET_TOKEN_NAME - token name to use for fetching token from Kibana. This requires Kibana configs to be set.
DEFAULT_FLEET_TOKEN_NAME - token name to use when [$FLEET_TOKEN_NAME] is not set. Defaults to "Default policy".
lykkin marked this conversation as resolved.
Show resolved Hide resolved
FLEET_TOKEN_POLICY_NAME - token policy name to use for fetching token from Kibana. This requires Kibana configs to be set.

* Bootstrapping Fleet Server
Expand All @@ -95,7 +96,8 @@ The following actions are possible and grouped based on the actions.
FLEET_SERVER_ELASTICSEARCH_CA_TRUSTED_FINGERPRINT - The sha-256 fingerprint value of the certificate authority to trust
FLEET_SERVER_ELASTICSEARCH_INSECURE - disables cert validation for communication with Elasticsearch
FLEET_SERVER_SERVICE_TOKEN - service token to use for communication with elasticsearch
FLEET_SERVER_POLICY_ID - policy ID for Fleet Server to use for itself ("Default Fleet Server policy" used when undefined)
FLEET_SERVER_POLICY_ID - policy ID for Fleet Server to use for itself ([$DEFAULT_FLEET_SERVER_POLICY_ID] used when undefined)
DEFAULT_FLEET_SERVER_POLICY_ID - policy ID for Fleet Server to use for itself when none is specified. (defaults to "fleet-server-policy")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't see the need for this option. Just make the default for FLEET_SERVER_POLICY_ID when one is not provided.

FLEET_SERVER_HOST - binding host for Fleet Server HTTP (overrides the policy). By default this is 0.0.0.0.
FLEET_SERVER_PORT - binding port for Fleet Server HTTP (overrides the policy)
FLEET_SERVER_CERT - path to certificate to use for HTTPS endpoint
Expand Down Expand Up @@ -146,6 +148,10 @@ func logError(streams *cli.IOStreams, err error) {
fmt.Fprintf(streams.Err, "Error: %v\n%s\n", err, troubleshootMessage())
}

func logWarn(streams *cli.IOStreams, a ...interface{}) {
fmt.Fprintln(streams.Out, append([]interface{}{"WARN: "}, a...)...)
}

func logInfo(streams *cli.IOStreams, a ...interface{}) {
fmt.Fprintln(streams.Out, a...)
}
Expand Down Expand Up @@ -498,7 +504,10 @@ func kibanaFetchPolicy(cfg setupConfig, client *kibana.Client, streams *cli.IOSt
if err != nil {
return nil, err
}
return findPolicy(cfg, policies.Items)
if err != nil {
return nil, err
}
return findPolicy(cfg, policies.Items, streams)
}

func kibanaFetchToken(cfg setupConfig, client *kibana.Client, policy *kibanaPolicy, streams *cli.IOStreams, tokenName string) (string, error) {
Expand Down Expand Up @@ -541,31 +550,46 @@ func kibanaClient(cfg kibanaConfig, headers map[string]string) (*kibana.Client,
}, 0, "Elastic-Agent")
}

func findPolicy(cfg setupConfig, policies []kibanaPolicy) (*kibanaPolicy, error) {
func findPolicy(cfg setupConfig, policies []kibanaPolicy, streams *cli.IOStreams) (*kibanaPolicy, error) {
policyID := ""
policyName := cfg.Fleet.TokenPolicyName
if cfg.FleetServer.Enable {
policyID = cfg.FleetServer.PolicyID
}

for _, policy := range policies {
if policyID != "" {
if policyID == policy.ID {
logInfo(streams, "using policy with id", policyID)
return &policy, nil
}
} else if policyName != "" {
if policyName == policy.Name {
logInfo(streams, "using policy named", policyName)
return &policy, nil
}
} else if cfg.FleetServer.Enable {
if policy.IsDefaultFleetServer {
if policy.ID == cfg.FleetServer.DefaultPolicyID {
logWarn(streams, "using default fleet policy id:", policy.ID)
return &policy, nil
}
} else {
if policy.IsDefault {
if policy.Name == cfg.Fleet.DefaultTokenPolicyName {
logWarn(streams, "using default policy name:", policy.Name)
return &policy, nil
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the added logging and making it explicit this will helps us.

}

if cfg.FleetServer.Enable {
if policyID == "" {
logWarn(streams, "No policy id set, please set the FLEET_SERVER_POLICY_ID environment variable to the fleet server policy id.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I missed part of a conversation, but whats the point of this warning? I believe that Fleet Server will find the policy correctly without an ID. So is a warning here really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's what i understand this code to mean:

this would be hit if the user didn't configure either a policy id or default when hosting a fleet server under the agent. this gives more specific information to the user assuming the agent fails to find a policy to get a token from.
this coincides with a path that halts start up of the agent.

i'll add comments for both of this situation, and the one below.

}
} else {
if policyName == "" {
logWarn(streams, "No policy name set, please set the FLEET_TOKEN_POLICY_NAME environment variable to the agent policy name.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Again can you give me more detail here? Maybe a code comment would be good to explain the flow here.

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@blakerouse I need to relearn the quirks of the enrollment process, but should we just error out?

return nil, fmt.Errorf(`unable to find policy named "%s"`, policyName)
}

Expand Down Expand Up @@ -899,11 +923,9 @@ func copyFile(destPath string, srcPath string, mode os.FileMode) error {
}

type kibanaPolicy struct {
ID string `json:"id"`
Name string `json:"name"`
Status string `json:"status"`
IsDefault bool `json:"is_default"`
IsDefaultFleetServer bool `json:"is_default_fleet_server"`
ID string `json:"id"`
Name string `json:"name"`
Status string `json:"status"`
}

type kibanaPolicies struct {
Expand Down
199 changes: 199 additions & 0 deletions x-pack/elastic-agent/pkg/agent/cmd/container_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package cmd

import (
"testing"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/cli"

"github.com/stretchr/testify/require"
)

var (
defaultFleetPolicy = kibanaPolicy{
ID: "fleet-server-policy",
Name: "Default Fleet Server policy",
Status: "active",
}
defaultAgentPolicy = kibanaPolicy{
ID: "2016d7cc-135e-5583-9758-3ba01f5a06e5",
Name: "Default policy",
Status: "active",
}
nondefaultAgentPolicy = kibanaPolicy{
ID: "bc634ea6-8460-4925-babd-7540c3e7df24",
Name: "Another free policy",
Status: "active",
}

nondefaultFleetPolicy = kibanaPolicy{
ID: "7b0093d2-7eab-4862-86c8-63b3dd1db001",
Name: "Some kinda dependent policy",
Status: "active",
}
)

var streams *cli.IOStreams = cli.NewIOStreams()

var policies kibanaPolicies = kibanaPolicies{
Items: []kibanaPolicy{
defaultFleetPolicy,
defaultAgentPolicy,
nondefaultAgentPolicy,
nondefaultFleetPolicy,
},
}

// Finding policies

func TestFindPolicyById(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: true,
PolicyID: "7b0093d2-7eab-4862-86c8-63b3dd1db001",
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.NoError(t, err)
require.Equal(t, &nondefaultFleetPolicy, policy)
}

func TestFindPolicyByName(t *testing.T) {
cfg := setupConfig{
Fleet: fleetConfig{
TokenPolicyName: "Default policy",
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.NoError(t, err)
require.Equal(t, &defaultAgentPolicy, policy)
}

func TestFindPolicyByIdOverName(t *testing.T) {
cfg := setupConfig{
Fleet: fleetConfig{
TokenPolicyName: "Default policy",
},
FleetServer: fleetServerConfig{
Enable: true,
PolicyID: "7b0093d2-7eab-4862-86c8-63b3dd1db001",
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.NoError(t, err)
require.Equal(t, &nondefaultFleetPolicy, policy)
}

func TestFindPolicyByIdMiss(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: true,
PolicyID: "invalid id",
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.Error(t, err)
require.Nil(t, policy)
}

func TestFindPolicyByNameMiss(t *testing.T) {
cfg := setupConfig{
Fleet: fleetConfig{
TokenPolicyName: "invalid name",
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.Error(t, err)
require.Nil(t, policy)
}

func TestFindPolicyDefaultFleet(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: true,
DefaultPolicyID: "fleet-server-policy",
},
}

items := []kibanaPolicy{
defaultAgentPolicy,
nondefaultAgentPolicy,
nondefaultFleetPolicy,
defaultFleetPolicy,
}

policy, err := findPolicy(cfg, items, streams)
require.NoError(t, err)
require.Equal(t, &defaultFleetPolicy, policy)
}

func TestFindPolicyAmbiguousNoDefaultFleet(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: true,
},
}

items := []kibanaPolicy{
defaultAgentPolicy,
nondefaultAgentPolicy,
nondefaultFleetPolicy,
defaultFleetPolicy,
}

policy, err := findPolicy(cfg, items, streams)
require.Error(t, err)
require.Nil(t, policy)
}

func TestFindPolicyDefaultNonFleet(t *testing.T) {
cfg := setupConfig{
Fleet: fleetConfig{
DefaultTokenPolicyName: "Default policy",
},
FleetServer: fleetServerConfig{
Enable: false,
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.NoError(t, err)
require.Equal(t, &defaultAgentPolicy, policy)
}

func TestFindPolicyNoMatchNonFleet(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: false,
},
}

policy, err := findPolicy(cfg, policies.Items, streams)
require.Error(t, err)
require.Nil(t, policy)
}

func TestFindPolicyNoMatchFleet(t *testing.T) {
cfg := setupConfig{
FleetServer: fleetServerConfig{
Enable: true,
},
}

items := []kibanaPolicy{
defaultAgentPolicy,
nondefaultAgentPolicy,
nondefaultFleetPolicy,
}
policy, err := findPolicy(cfg, items, streams)
require.Error(t, err)
require.Nil(t, policy)
}
Loading