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

[Elastic-Agent] Use internal port for local fleet server #28993

Merged
merged 7 commits into from
Nov 23, 2021
Merged

[Elastic-Agent] Use internal port for local fleet server #28993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f223462
use itnernal port for local fleet server
michalpristas Nov 15, 2021
cfcc3f0
Merge branch 'master' of github.com:elastic/beats into use-secondary-…
michalpristas Nov 16, 2021
f9a3e4b
internal on localhost
michalpristas Nov 18, 2021
fa9f592
remove unwanted exports from spec
michalpristas Nov 19, 2021
3607c83
fixed override
michalpristas Nov 19, 2021
27400e8
log message
michalpristas Nov 19, 2021
778c564
log level
michalpristas Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x-pack/elastic-agent/pkg/agent/cmd/enroll.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ func enroll(streams *cli.IOStreams, cmd *cobra.Command, args []string) error {
fPolicy, _ := cmd.Flags().GetString("fleet-server-policy")
fHost, _ := cmd.Flags().GetString("fleet-server-host")
fPort, _ := cmd.Flags().GetUint16("fleet-server-port")
fInternalPort, _ := cmd.Flags().GetUint16("fleet-server-internal-port")
fCert, _ := cmd.Flags().GetString("fleet-server-cert")
fCertKey, _ := cmd.Flags().GetString("fleet-server-cert-key")
fInsecure, _ := cmd.Flags().GetBool("fleet-server-insecure-http")
Expand Down Expand Up @@ -336,6 +337,7 @@ func enroll(streams *cli.IOStreams, cmd *cobra.Command, args []string) error {
SpawnAgent: !fromInstall,
Headers: mapFromEnvList(fHeaders),
Timeout: fTimeout,
InternalPort: fInternalPort,
},
}

Expand Down
37 changes: 29 additions & 8 deletions x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ import (
)

const (
maxRetriesstoreAgentInfo = 5
waitingForAgent = "Waiting for Elastic Agent to start"
waitingForFleetServer = "Waiting for Elastic Agent to start Fleet Server"
defaultFleetServerHost = "0.0.0.0"
defaultFleetServerPort = 8220
maxRetriesstoreAgentInfo = 5
waitingForAgent = "Waiting for Elastic Agent to start"
waitingForFleetServer = "Waiting for Elastic Agent to start Fleet Server"
defaultFleetServerHost = "0.0.0.0"
defaultFleetServerPort = 8220
defaultFleetServerInternalHost = "localhost"
defaultFleetServerInternalPort = 8221
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets make sure we document this internal as needed by fleet-server. @jlind23 Can you make sure we follow up on this? We also need to have this in the changelog as if by chance someone uses this port already on the machine, it would break things (hopefully unlikely).

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ruflin Where should it be documented though?

Copy link
Contributor

Choose a reason for hiding this comment

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

Two ideas:

@bmorelli25 Where would these internal docs around elastic-agent and fleet-server fit best?

Copy link
Member

Choose a reason for hiding this comment

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

This is @dedemorton's territory. DeDe, can you take a look?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so it sounds like you don't want to document the internal port setting in the main docs (command reference + Fleet Server settings) because users will not set it in a production environment. It also sounds like we're covered for the situation where the port is already in use (the code will select an unused port when there's a conflict).

I'd like to understand specific use cases for this setting, though, because I'm always suspicious of hidden settings that we don't want to document for users. We have a lot of users who are on the support team and might look in the reference first. My preference is to have reference docs that are complete, but to make the usage clear in the description.

Sounds like it also makes sense to cover this setting in the troubleshooting docs, but we need to explain when/why to change this setting during testing.

We could also mention in the Fleet Server docs that Fleet Server uses port 8221 internally, but will look for another port if that one isn't available.

Copy link
Contributor

Choose a reason for hiding this comment

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

It will not look for antoher port. We might want to use the port for testing purpose or as an escape hatch if indeed a user has a conflict.

There might we 2 things we should document:

  • Architecture / Internals: What port is used for internal communication -> public
  • Hidden config to change it -> developer focus, mentioned that not officially supported

)

var (
Expand Down Expand Up @@ -80,6 +82,7 @@ type enrollCmdFleetServerOption struct {
PolicyID string
Host string
Port uint16
InternalPort uint16
Cert string
CertKey string
Insecure bool
Expand All @@ -91,6 +94,7 @@ type enrollCmdFleetServerOption struct {
// enrollCmdOption define all the supported enrollment option.
type enrollCmdOption struct {
URL string `yaml:"url,omitempty"`
InternalURL string `yaml:"-"`
CAs []string `yaml:"ca,omitempty"`
CASha256 []string `yaml:"ca_sha256,omitempty"`
Insecure bool `yaml:"insecure,omitempty"`
Expand Down Expand Up @@ -306,7 +310,7 @@ func (c *enrollCmd) fleetServerBootstrap(ctx context.Context, persistentConfig m
fleetConfig, err := createFleetServerBootstrapConfig(
c.options.FleetServer.ConnStr, c.options.FleetServer.ServiceToken,
c.options.FleetServer.PolicyID,
c.options.FleetServer.Host, c.options.FleetServer.Port,
c.options.FleetServer.Host, c.options.FleetServer.Port, c.options.FleetServer.InternalPort,
c.options.FleetServer.Cert, c.options.FleetServer.CertKey, c.options.FleetServer.ElasticsearchCA,
c.options.FleetServer.Headers,
c.options.ProxyURL,
Expand Down Expand Up @@ -401,6 +405,14 @@ func (c *enrollCmd) prepareFleetTLS() error {
if c.options.URL == "" {
return errors.New("url is required when a certificate is provided")
}

if c.options.FleetServer.InternalPort > 0 {
if c.options.FleetServer.InternalPort != defaultFleetServerInternalPort {
c.log.Warnf("Internal endpoint configured to: %d. Changing this value is not supported.", c.options.FleetServer.InternalPort)
}
c.options.InternalURL = fmt.Sprintf("%s:%d", defaultFleetServerInternalHost, c.options.FleetServer.InternalPort)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a long entry here which internal port is used and that this is not "officially" supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can put it there, but as soon as you have something on a machine which binds this port there needs to be official workaround and i think this is it

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets wait until this happens. Having the log message and user pinging us on it will also help us detect if this might become a common problem and the port we selected is not ideal.

}

return nil
}

Expand Down Expand Up @@ -504,7 +516,7 @@ func (c *enrollCmd) enroll(ctx context.Context, persistentConfig map[string]inte
serverConfig, err := createFleetServerBootstrapConfig(
c.options.FleetServer.ConnStr, c.options.FleetServer.ServiceToken,
c.options.FleetServer.PolicyID,
c.options.FleetServer.Host, c.options.FleetServer.Port,
c.options.FleetServer.Host, c.options.FleetServer.Port, c.options.FleetServer.InternalPort,
c.options.FleetServer.Cert, c.options.FleetServer.CertKey, c.options.FleetServer.ElasticsearchCA,
c.options.FleetServer.Headers,
c.options.ProxyURL, c.options.ProxyDisabled, c.options.ProxyHeaders,
Expand All @@ -516,6 +528,10 @@ func (c *enrollCmd) enroll(ctx context.Context, persistentConfig map[string]inte
// no longer need bootstrap at this point
serverConfig.Server.Bootstrap = false
fleetConfig.Server = serverConfig.Server
// use internal URL for future requests
if c.options.InternalURL != "" {
fleetConfig.Client.Host = c.options.InternalURL
}
}

configToStore := map[string]interface{}{
Expand Down Expand Up @@ -836,7 +852,7 @@ func storeAgentInfo(s saver, reader io.Reader) error {

func createFleetServerBootstrapConfig(
connStr, serviceToken, policyID, host string,
port uint16,
port uint16, internalPort uint16,
cert, key, esCA string,
headers map[string]string,
proxyURL string,
Expand Down Expand Up @@ -865,6 +881,9 @@ func createFleetServerBootstrapConfig(
if port == 0 {
port = defaultFleetServerPort
}
if internalPort == 0 {
internalPort = defaultFleetServerInternalPort
}
if len(headers) > 0 {
if es.Headers == nil {
es.Headers = make(map[string]string)
Expand All @@ -888,6 +907,7 @@ func createFleetServerBootstrapConfig(
Host: host,
Port: port,
}

if policyID != "" {
cfg.Server.Policy = &configuration.FleetServerPolicyConfig{ID: policyID}
}
Expand All @@ -905,6 +925,7 @@ func createFleetServerBootstrapConfig(

if localFleetServer {
cfg.Client.Transport.Proxy.Disable = true
cfg.Server.InternalPort = internalPort
}

if err := cfg.Valid(); err != nil {
Expand Down
13 changes: 7 additions & 6 deletions x-pack/elastic-agent/pkg/agent/configuration/fleet_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import (

// FleetServerConfig is the configuration written so Elastic Agent can run Fleet Server.
type FleetServerConfig struct {
Bootstrap bool `config:"bootstrap" yaml:"bootstrap,omitempty"`
Policy *FleetServerPolicyConfig `config:"policy" yaml:"policy,omitempty"`
Output FleetServerOutputConfig `config:"output" yaml:"output,omitempty"`
Host string `config:"host" yaml:"host,omitempty"`
Port uint16 `config:"port" yaml:"port,omitempty"`
TLS *tlscommon.Config `config:"ssl" yaml:"ssl,omitempty"`
Bootstrap bool `config:"bootstrap" yaml:"bootstrap,omitempty"`
Policy *FleetServerPolicyConfig `config:"policy" yaml:"policy,omitempty"`
Output FleetServerOutputConfig `config:"output" yaml:"output,omitempty"`
Host string `config:"host" yaml:"host,omitempty"`
Port uint16 `config:"port" yaml:"port,omitempty"`
InternalPort uint16 `config:"internal_port" yaml:"internal_port,omitempty"`
TLS *tlscommon.Config `config:"ssl" yaml:"ssl,omitempty"`
}

// FleetServerPolicyConfig is the configuration for the policy Fleet Server should run on.
Expand Down
2 changes: 1 addition & 1 deletion x-pack/elastic-agent/pkg/agent/program/supported.go
View file
Open in desktop

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

1 change: 1 addition & 0 deletions x-pack/elastic-agent/spec/fleet-server.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ rules:
selectors:
- fleet.server.host
- fleet.server.port
- fleet.server.internal_port
- fleet.server.ssl
path: inputs.0.server

Expand Down