luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

acl: sso auth methods RPC/API/CLI should return created or updated objects (#15410) 

Currently CRUD code that operates on SSO auth methods does not return created or updated object upon creation/update. This is bad UX and inconsistent behavior compared to other ACL objects like roles, policies or tokens.

This PR fixes it.

Relates to #13120
This commit is contained in:
Piotr Kazmierczak 2022-11-29 07:36:36 +01:00 committed by GitHub
parent a65fbeb3b3
commit 0eccd3286c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 54 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
api/acl.go
View File

@ -319,27 +319,29 @@ func (a *ACLAuthMethods) List(q *QueryOptions) ([]*ACLAuthMethodListStub, *Query
}
// Create is used to create an ACL auth-method.
func (a *ACLAuthMethods) Create(authMethod *ACLAuthMethod, w *WriteOptions) (*WriteMeta, error) {
func (a *ACLAuthMethods) Create(authMethod *ACLAuthMethod, w *WriteOptions) (*ACLAuthMethod, *WriteMeta, error) {
if authMethod.Name == "" {
return nil, errMissingACLAuthMethodName
return nil, nil, errMissingACLAuthMethodName
}
wm, err := a.client.write("/v1/acl/auth-method", authMethod, nil, w)
var resp ACLAuthMethod
wm, err := a.client.write("/v1/acl/auth-method", authMethod, &resp, w)
if err != nil {
return nil, err
return nil, nil, err
}
return wm, nil
return &resp, wm, nil
}
// Update is used to update an existing ACL auth-method.
func (a *ACLAuthMethods) Update(authMethod *ACLAuthMethod, w *WriteOptions) (*WriteMeta, error) {
func (a *ACLAuthMethods) Update(authMethod *ACLAuthMethod, w *WriteOptions) (*ACLAuthMethod, *WriteMeta, error) {
if authMethod.Name == "" {
return nil, errMissingACLAuthMethodName
return nil, nil, errMissingACLAuthMethodName
}
wm, err := a.client.write("/v1/acl/auth-method/"+authMethod.Name, authMethod, nil, w)
var resp ACLAuthMethod
wm, err := a.client.write("/v1/acl/auth-method/"+authMethod.Name, authMethod, &resp, w)
if err != nil {
return nil, err
return nil, nil, err
}
return wm, nil
return &resp, wm, nil
}
// Delete is used to delete an ACL auth-method.

6
api/acl_test.go
View File

@ -209,7 +209,7 @@ func TestACLTokens_CreateUpdate(t *testing.T) {
out2.Roles = []*ACLTokenRoleLink{{Name: aclRoleCreateResp.Name}}
out2.ExpirationTTL = 0
out3, writeMeta, err = at.Update(out2, nil)
out3, _, err = at.Update(out2, nil)
require.NoError(t, err)
require.NotNil(t, out3)
require.Len(t, out3.Policies, 1)
@ -608,7 +608,7 @@ func TestACLAuthMethods(t *testing.T) {
MaxTokenTTL: 15 * time.Minute,
Default: true,
}
writeMeta, err := testClient.ACLAuthMethods().Create(&authMethod, nil)
_, writeMeta, err := testClient.ACLAuthMethods().Create(&authMethod, nil)
must.NoError(t, err)
assertWriteMeta(t, writeMeta)
@ -632,7 +632,7 @@ func TestACLAuthMethods(t *testing.T) {
// Update the auth-method token locality.
authMethod.TokenLocality = ACLAuthMethodTokenLocalityGlobal
writeMeta, err = testClient.ACLAuthMethods().Update(&authMethod, nil)
_, writeMeta, err = testClient.ACLAuthMethods().Update(&authMethod, nil)
must.NoError(t, err)
assertWriteMeta(t, writeMeta)

4
command/acl_auth_method_create.go
View File

@ -168,12 +168,12 @@ func (a *ACLAuthMethodCreateCommand) Run(args []string) int {
}
// Create the auth method via the API.
_, err = client.ACLAuthMethods().Create(&authMethod, nil)
method, _, err := client.ACLAuthMethods().Create(&authMethod, nil)
if err != nil {
a.Ui.Error(fmt.Sprintf("Error creating ACL auth method: %v", err))
return 1
}
a.Ui.Output(fmt.Sprintf("Created ACL auth method %s", a.name))
a.Ui.Output(fmt.Sprintf("Created ACL auth method:\n%s", formatAuthMethod(method)))
return 0
}

4
command/acl_auth_method_update.go
View File

@ -185,13 +185,13 @@ func (a *ACLAuthMethodUpdateCommand) Run(args []string) int {
}
// Update the auth method via the API.
_, err = client.ACLAuthMethods().Update(&updatedMethod, nil)
method, _, err := client.ACLAuthMethods().Update(&updatedMethod, nil)
if err != nil {
a.Ui.Error(fmt.Sprintf("Error updating ACL auth method: %v", err))
return 1
}
a.Ui.Output(fmt.Sprintf("Updated ACL auth method %s", originalMethodName))
a.Ui.Output(fmt.Sprintf("Updated ACL auth method:\n%s", formatAuthMethod(method)))
return 0
}

4
command/agent/acl_endpoint.go
View File

@ -670,5 +670,9 @@ func (s *HTTPServer) aclAuthMethodUpsertRequest(
return nil, err
}
setIndex(resp, out.Index)
if len(out.AuthMethods) > 0 {
return out.AuthMethods[0], nil
}
return nil, nil
}

24
command/agent/acl_endpoint_test.go
View File

@ -1104,14 +1104,14 @@ func TestHTTPServer_ACLAuthMethodRequest(t *testing.T) {
// Build the HTTP request.
req, err := http.NewRequest(http.MethodPut, "/v1/acl/auth-method", encodeReq(mockACLRole))
require.NoError(t, err)
must.NoError(t, err)
respW := httptest.NewRecorder()
// Send the HTTP request.
obj, err := srv.Server.ACLAuthMethodRequest(respW, req)
require.Error(t, err)
require.ErrorContains(t, err, "Permission denied")
require.Nil(t, obj)
must.Error(t, err)
must.StrContains(t, err.Error(), "Permission denied")
must.Nil(t, obj)
},
},
{
@ -1120,7 +1120,7 @@ func TestHTTPServer_ACLAuthMethodRequest(t *testing.T) {
// Build the HTTP request.
req, err := http.NewRequest(http.MethodConnect, "/v1/acl/auth-method", nil)
require.NoError(t, err)
must.NoError(t, err)
respW := httptest.NewRecorder()
// Ensure we have a token set.
@ -1128,9 +1128,9 @@ func TestHTTPServer_ACLAuthMethodRequest(t *testing.T) {
// Send the HTTP request.
obj, err := srv.Server.ACLAuthMethodRequest(respW, req)
require.Error(t, err)
require.ErrorContains(t, err, "Invalid method")
require.Nil(t, obj)
must.Error(t, err)
must.StrContains(t, err.Error(), "Invalid method")
must.Nil(t, obj)
},
},
{
@ -1142,7 +1142,7 @@ func TestHTTPServer_ACLAuthMethodRequest(t *testing.T) {
// Build the HTTP request.
req, err := http.NewRequest(http.MethodPut, "/v1/acl/auth-method", encodeReq(mockACLAuthMethod))
require.NoError(t, err)
must.NoError(t, err)
respW := httptest.NewRecorder()
// Ensure we have a token set.
@ -1150,8 +1150,10 @@ func TestHTTPServer_ACLAuthMethodRequest(t *testing.T) {
// Send the HTTP request.
obj, err := srv.Server.ACLAuthMethodRequest(respW, req)
require.NoError(t, err)
require.Nil(t, obj)
must.NoError(t, err)
result := obj.(*structs.ACLAuthMethod)
must.Eq(t, result, mockACLAuthMethod)
},
},
}

16
nomad/acl_endpoint.go
View File

@ -1724,6 +1724,22 @@ func (a *ACL) UpsertAuthMethods(
return err
}
// Populate the response. We do a lookup against the state to pick up the
// proper create / modify times.
stateSnapshot, err := a.srv.State().Snapshot()
if err != nil {
return err
}
for _, method := range args.AuthMethods {
lookupAuthMethod, err := stateSnapshot.GetACLAuthMethodByName(nil, method.Name)
if err != nil {
return structs.NewErrRPCCodedf(400, "ACL auth method lookup failed: %v", err)
}
if lookupAuthMethod != nil {
reply.AuthMethods = append(reply.AuthMethods, lookupAuthMethod)
}
}
// Update the index
reply.Index = index
return nil

1
nomad/acl_endpoint_test.go
View File

@ -3041,4 +3041,5 @@ func TestACLEndpoint_UpsertACLAuthMethods(t *testing.T) {
out, err := s1.fsm.State().GetACLAuthMethodByName(nil, am1.Name)
must.Nil(t, err)
must.NotNil(t, out)
must.True(t, am1.Equal(resp.AuthMethods[0]))
}

1
nomad/structs/acl.go
View File

@ -825,6 +825,7 @@ type ACLAuthMethodUpsertRequest struct {
// ACLAuthMethodUpsertResponse is a response of the upsert ACL auth methods
// operation
type ACLAuthMethodUpsertResponse struct {
AuthMethods []*ACLAuthMethod
WriteMeta
}