-
Notifications
You must be signed in to change notification settings - Fork 518
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
[ACM] Agent remote configuration management - Main functionality #2095
Closed
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
5a61845
Proof of concept for agent remote configuration
jalvz 64337b6
use Kibana config in apm-server namespace
jalvz be43167
override relevant vendored code with next beats update content
jalvz 978af63
remove the top level metatada attribute
jalvz 5d3a467
return everything as string
jalvz 3e163b2
add unit tests
jalvz a1ad347
make the linter happy
jalvz 4ce5d0a
code review comments
jalvz c097fc9
change endpoint name (code review)
jalvz 58f7533
Merge remote-tracking branch 'elastic/master' into cm-poc
jalvz c1f7a00
Merge remote-tracking branch 'elastic/master' into cm-poc
jalvz b3fc426
code review comments
jalvz 8ee337f
more code reviews
jalvz 475d0bf
Merge remote-tracking branch 'elastic/master' into cm-poc
jalvz 93f5a86
fix tests
jalvz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
// Licensed to Elasticsearch B.V. under one or more contributor | ||
// license agreements. See the NOTICE file distributed with | ||
// this work for additional information regarding copyright | ||
// ownership. Elasticsearch B.V. licenses this file to you under | ||
// the Apache License, Version 2.0 (the "License"); you may | ||
// not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, | ||
// software distributed under the License is distributed on an | ||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
package agentcfg | ||
|
||
import ( | ||
"fmt" | ||
"io" | ||
"io/ioutil" | ||
"net/http" | ||
|
||
"github.com/pkg/errors" | ||
|
||
"github.com/elastic/beats/libbeat/common" | ||
|
||
"github.com/elastic/apm-server/convert" | ||
"github.com/elastic/beats/libbeat/kibana" | ||
) | ||
|
||
const endpoint = "/api/apm/settings/cm/search" | ||
|
||
var minVersion = common.Version{Major: 7, Minor: 3} | ||
|
||
// Fetch retrieves agent configuration from Kibana | ||
func Fetch(kbClient *kibana.Client, q Query, err error) (map[string]string, string, error) { | ||
var doc Doc | ||
resultBytes, err := request(kbClient, convert.ToReader(q), err) | ||
err = convert.FromBytes(resultBytes, &doc, err) | ||
return doc.Source.Settings, doc.ID, err | ||
} | ||
|
||
func request(kbClient *kibana.Client, r io.Reader, err error) ([]byte, error) { | ||
if err != nil { | ||
return nil, err | ||
} | ||
if kbClient == nil { | ||
return nil, errors.New("No configured Kibana Client: provide apm-server.kibana.* settings") | ||
} | ||
if version := kbClient.GetVersion(); version.LessThan(&minVersion) { | ||
return nil, errors.New(fmt.Sprintf("Needs Kibana version %s or higher", minVersion.String())) | ||
} | ||
resp, err := kbClient.Send(http.MethodPost, endpoint, nil, nil, r) | ||
if err != nil { | ||
return nil, err | ||
} | ||
defer resp.Body.Close() | ||
|
||
result, err := ioutil.ReadAll(resp.Body) | ||
if resp.StatusCode >= http.StatusMultipleChoices { | ||
return nil, errors.New(string(result)) | ||
} | ||
return result, err | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Licensed to Elasticsearch B.V. under one or more contributor | ||
// license agreements. See the NOTICE file distributed with | ||
// this work for additional information regarding copyright | ||
// ownership. Elasticsearch B.V. licenses this file to you under | ||
// the Apache License, Version 2.0 (the "License"); you may | ||
// not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, | ||
// software distributed under the License is distributed on an | ||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
package agentcfg | ||
|
||
import ( | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/elastic/apm-server/tests" | ||
"github.com/elastic/beats/libbeat/kibana" | ||
) | ||
|
||
type m map[string]interface{} | ||
|
||
var q = Query{} | ||
|
||
func TestFetchNoClient(t *testing.T) { | ||
kb, kerr := kibana.NewKibanaClient(nil) | ||
_, _, ferr := Fetch(kb, q, kerr) | ||
require.Error(t, ferr) | ||
assert.Equal(t, ferr, kerr, kerr) | ||
} | ||
|
||
func TestFetchStringConversion(t *testing.T) { | ||
kb := tests.MockKibana(http.StatusOK, | ||
m{ | ||
"_id": "1", | ||
"_source": m{ | ||
"settings": m{ | ||
"sampling_rate": 0.5, | ||
}, | ||
}, | ||
}) | ||
result, etag, err := Fetch(kb, q, nil) | ||
require.NoError(t, err) | ||
assert.Equal(t, "1", etag, etag) | ||
assert.Equal(t, map[string]string{"sampling_rate": "0.5"}, result, result) | ||
} | ||
|
||
func TestFetchVersionCheck(t *testing.T) { | ||
kb := tests.MockKibana(http.StatusOK, m{}) | ||
kb.Connection.Version.Major = 6 | ||
_, _, err := Fetch(kb, q, nil) | ||
require.Error(t, err) | ||
assert.Contains(t, err.Error(), "version") | ||
} | ||
|
||
func TestFetchError(t *testing.T) { | ||
kb := tests.MockKibana(http.StatusNotFound, m{"error": "an error"}) | ||
_, _, err := Fetch(kb, q, nil) | ||
require.Error(t, err) | ||
assert.Equal(t, err.Error(), "{\"error\":\"an error\"}") | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
// Licensed to Elasticsearch B.V. under one or more contributor | ||
// license agreements. See the NOTICE file distributed with | ||
// this work for additional information regarding copyright | ||
// ownership. Elasticsearch B.V. licenses this file to you under | ||
// the Apache License, Version 2.0 (the "License"); you may | ||
// not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, | ||
// software distributed under the License is distributed on an | ||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
package agentcfg | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
) | ||
|
||
const ( | ||
// ServiceName keyword | ||
ServiceName = "service.name" | ||
// ServiceEnv keyword | ||
ServiceEnv = "service.environment" | ||
simitt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
// Doc represents an elasticsearch document | ||
type Doc struct { | ||
ID string `json:"_id"` | ||
Source Source `json:"_source"` | ||
} | ||
|
||
// Source represents the elasticsearch _source field of a document | ||
type Source struct { | ||
Settings Settings `json:"settings"` | ||
} | ||
|
||
// Settings hold agent configuration | ||
type Settings map[string]string | ||
simitt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// UnmarshalJSON overrides default method to convert any JSON type to string | ||
func (s *Settings) UnmarshalJSON(b []byte) error { | ||
in := make(map[string]interface{}) | ||
out := make(map[string]string) | ||
err := json.Unmarshal(b, &in) | ||
for k, v := range in { | ||
out[k] = fmt.Sprintf("%v", v) | ||
} | ||
*s = out | ||
return err | ||
} | ||
|
||
// NewQuery creates a Query struct | ||
func NewQuery(name, env string) Query { | ||
return Query{Service{name, env}} | ||
} | ||
|
||
// Query represents an URL body or query params for agent configuration | ||
type Query struct { | ||
Service Service `json:"service"` | ||
} | ||
|
||
// Service holds supported attributes for querying configuration | ||
type Service struct { | ||
Name string `json:"name"` | ||
Environment string `json:"environment,omitempty"` | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has there been a discussion around using the existing kibana settings instead of introducing a new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it has been discussed.
This is the same setting, just in a different namespace.
The beats setting is under
setup
which is not correct in our case; this is not for a setup command.Also, should we use that, the server would refuse to start if Kibana is not reachable, which is not what we want, and not coherent eg. with Elasticsearch output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha 👍