Skip to content

Commit

Permalink
PMM-11714 Don't return error if API key can't be created.
Browse files Browse the repository at this point in the history
  • Loading branch information
BupycHuk committed Sep 7, 2023
1 parent e38877a commit ba37ba0
Show file tree
Hide file tree
Showing 22 changed files with 153 additions and 671 deletions.
5 changes: 5 additions & 0 deletions admin/commands/management/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@ var registerResultT = commands.ParseTemplate(`
pmm-agent registered.
pmm-agent ID: {{ .PMMAgent.AgentID }}
Node ID : {{ .PMMAgent.RunsOnNodeID }}
{{ if .Warning }}
Warning: {{ .Warning }}
{{- end -}}
`)

type registerResult struct {
GenericNode *node.RegisterNodeOKBodyGenericNode `json:"generic_node"`
ContainerNode *node.RegisterNodeOKBodyContainerNode `json:"container_node"`
PMMAgent *node.RegisterNodeOKBodyPMMAgent `json:"pmm_agent"`
Warning string `json:"warning"`
}

func (res *registerResult) Result() {}
Expand Down Expand Up @@ -96,5 +100,6 @@ func (cmd *RegisterCommand) RunCmd() (commands.Result, error) {
GenericNode: resp.Payload.GenericNode,
ContainerNode: resp.Payload.ContainerNode,
PMMAgent: resp.Payload.PMMAgent,
Warning: resp.Payload.Warning,
}, nil
}
51 changes: 51 additions & 0 deletions admin/commands/management/register_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package management

import (
"github.com/percona/pmm/api/managementpb/json/client/node"
"github.com/stretchr/testify/assert"
"testing"
)

func TestRegisterResult(t *testing.T) {
tests := []struct {
name string
result registerResult
want string
}{
{
name: "Success",
result: registerResult{
PMMAgent: &node.RegisterNodeOKBodyPMMAgent{
AgentID: "/agent_id/new_id",
RunsOnNodeID: "/node_id/second_id",
},
Warning: "",
},
want: `pmm-agent registered.
pmm-agent ID: /agent_id/new_id
Node ID : /node_id/second_id
`,
},
{
name: "With warning",
result: registerResult{
PMMAgent: &node.RegisterNodeOKBodyPMMAgent{
AgentID: "/agent_id/warning",
RunsOnNodeID: "/node_id/warning_node",
},
Warning: "Couldn't create Admin API Key",
},
want: `pmm-agent registered.
pmm-agent ID: /agent_id/warning
Node ID : /node_id/warning_node
Warning: Couldn't create Admin API Key
`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, tt.result.String(), "String()")
})
}
}
3 changes: 3 additions & 0 deletions api/managementpb/json/client/node/register_node_responses.go

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

5 changes: 5 additions & 0 deletions api/managementpb/json/managementpb.json
Original file line number Diff line number Diff line change
Expand Up @@ -3889,6 +3889,11 @@
"description": "Token represents token for vmagent auth config.",
"type": "string",
"x-order": 3
},
"warning": {
"description": "Warning message.",
"type": "string",
"x-order": 4
}
}
}
Expand Down
57 changes: 34 additions & 23 deletions api/managementpb/node.pb.go

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

2 changes: 2 additions & 0 deletions api/managementpb/node.pb.validate.go

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

2 changes: 2 additions & 0 deletions api/managementpb/node.proto
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ message RegisterNodeResponse {
inventory.PMMAgent pmm_agent = 3;
// Token represents token for vmagent auth config.
string token = 4;
// Warning message.
string warning = 5;
}

// Node service provides public Management API methods for Nodes.
Expand Down
5 changes: 5 additions & 0 deletions api/swagger/swagger-dev.json
Original file line number Diff line number Diff line change
Expand Up @@ -26884,6 +26884,11 @@
"description": "Token represents token for vmagent auth config.",
"type": "string",
"x-order": 3
},
"warning": {
"description": "Warning message.",
"type": "string",
"x-order": 4
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions api/swagger/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -18053,6 +18053,11 @@
"description": "Token represents token for vmagent auth config.",
"type": "string",
"x-order": 3
},
"warning": {
"description": "Warning message.",
"type": "string",
"x-order": 4
}
}
}
Expand Down
23 changes: 14 additions & 9 deletions managed/services/grafana/auth_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,15 +533,7 @@ func cleanPath(p string) (string, error) {

func (s *AuthServer) getAuthUser(ctx context.Context, req *http.Request, l *logrus.Entry) (*authUser, *authError) {
// check Grafana with some headers from request
authHeaders := make(http.Header)
for _, k := range []string{
"Authorization",
"Cookie",
} {
if v := req.Header.Get(k); v != "" {
authHeaders.Set(k, v)
}
}
authHeaders := s.authHeaders(req)
j, err := json.Marshal(authHeaders)
if err != nil {
l.Warnf("%s", err)
Expand All @@ -558,6 +550,19 @@ func (s *AuthServer) getAuthUser(ctx context.Context, req *http.Request, l *logr
return s.retrieveRole(ctx, hash, authHeaders, l)
}

func (s *AuthServer) authHeaders(req *http.Request) http.Header {
authHeaders := make(http.Header)
for _, k := range []string{
"Authorization",
"Cookie",
} {
if v := req.Header.Get(k); v != "" {
authHeaders.Set(k, v)
}
}
return authHeaders
}

func (s *AuthServer) retrieveRole(ctx context.Context, hash string, authHeaders http.Header, l *logrus.Entry) (*authUser, *authError) {
authUser, err := s.c.getAuthUser(ctx, authHeaders)
if err != nil {
Expand Down
6 changes: 4 additions & 2 deletions managed/services/grafana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (c *Client) GetUserID(ctx context.Context) (int, error) {
// Ctx is used only for cancelation.
func (c *Client) getAuthUser(ctx context.Context, authHeaders http.Header) (authUser, error) {
// Check if it's API Key
if c.isAPIKeyAuth(authHeaders.Get("Authorization")) {
if c.IsAPIKeyAuth(authHeaders) {
role, err := c.getRoleForAPIKey(ctx, authHeaders)
return authUser{
role: role,
Expand Down Expand Up @@ -277,7 +277,9 @@ func (c *Client) getAuthUser(ctx context.Context, authHeaders http.Header) (auth
}, nil
}

func (c *Client) isAPIKeyAuth(authHeader string) bool {
// IsAPIKeyAuth checks if the request made using API Key.
func (c *Client) IsAPIKeyAuth(headers http.Header) bool {
authHeader := headers.Get("Authorization")
switch {
case strings.HasPrefix(authHeader, "Bearer"):
return true
Expand Down
48 changes: 0 additions & 48 deletions managed/services/management/mock_agents_registry_test.go

This file was deleted.

34 changes: 0 additions & 34 deletions managed/services/management/mock_agents_state_updater_test.go

This file was deleted.

Loading

0 comments on commit ba37ba0

Please sign in to comment.