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

Member Management Part2 #872

Merged
merged 34 commits into from
Dec 18, 2020
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
cb8afd6
A few minor fixes and some cosmetic changes to members management
holgerkoser Oct 29, 2020
59bb3ce
read only projectName from cache
holgerkoser Oct 29, 2020
8e570c1
add member description property
holgerkoser Oct 30, 2020
6d5d5d7
PR review I
holgerkoser Oct 30, 2020
331cdf5
Added rotate service account secret feature
grolu Nov 3, 2020
90685b6
Merge branch 'master' into enh/member-management-4
grolu Nov 3, 2020
5d88cd7
Merge branch 'master' into enh/member-management-4
grolu Nov 3, 2020
e6a7d44
Use data table for service account list
grolu Nov 4, 2020
aab5b4f
use data table for user account list
grolu Nov 5, 2020
a28323a
small fixes
grolu Nov 6, 2020
020edbb
fixed unused var
grolu Nov 6, 2020
500d4b1
Merge branch 'master' into enh/member-management-4
grolu Nov 11, 2020
1244108
Apply suggestions from code review
grolu Nov 12, 2020
158c55a
PR Feedback #1
grolu Nov 12, 2020
2d2bb60
PR feedback #2
grolu Nov 12, 2020
3452886
row alignment
grolu Nov 13, 2020
0b64a22
Apply suggestions from code review
grolu Nov 13, 2020
ad1217f
PR Feedback #3
grolu Nov 13, 2020
d34fd28
Small changes
grolu Nov 16, 2020
a0feb76
Added sorting capabilities and limit description height in member table
grolu Nov 19, 2020
5a93d4f
Merge branch 'master' into enh/member-management-4
grolu Dec 4, 2020
1976000
fix column sorting
grolu Dec 7, 2020
9b8b538
Refactored table column selection into re-usable component
grolu Dec 8, 2020
25723d7
make columns selectable & resetable + local storage
grolu Dec 9, 2020
a386916
Apply suggestions from code review
grolu Dec 14, 2020
020ab9e
Merge branch 'master' into enh/member-management-4
grolu Dec 16, 2020
401f95b
changes as discussed
grolu Dec 16, 2020
28cc300
PR Feedback
grolu Dec 17, 2020
4800c13
Added handling for members where serviceaccount has been removed
grolu Dec 17, 2020
6ebe507
Apply suggestions from code review
grolu Dec 17, 2020
c71969a
updated member snapshot file
grolu Dec 17, 2020
6303201
Merge branch 'master' into enh/member-management-4
grolu Dec 17, 2020
ebb5ce0
Fixed lint errors
grolu Dec 18, 2020
f1d8fa2
Orphaned Robot Members (#914)
holgerkoser Dec 18, 2020
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
18 changes: 18 additions & 0 deletions backend/lib/routes/members.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

const express = require('express')
const { members } = require('../services')
const { UnprocessableEntity } = require('http-errors')

const router = module.exports = express.Router({
mergeParams: true
Expand Down Expand Up @@ -56,6 +57,23 @@ router.route('/:name')
next(err)
}
})
.post(async (req, res, next) => {
try {
const user = req.user
const { namespace, name } = req.params
const { method } = req.body

switch (method) {
case 'rotateSecret':
res.send(await members.rotateSecret({ user, namespace, name }))
grolu marked this conversation as resolved.
Show resolved Hide resolved
break
default:
throw new UnprocessableEntity(`${method} not allowed for members`)
}
} catch (err) {
next(err)
}
})
.delete(async (req, res, next) => {
try {
const user = req.user
Expand Down
3 changes: 2 additions & 1 deletion backend/lib/routes/terminals.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
const express = require('express')
const { terminals, authorization } = require('../services')
const _ = require('lodash')
const { UnprocessableEntity } = require('http-errors')

const router = module.exports = express.Router({
mergeParams: true
Expand Down Expand Up @@ -37,7 +38,7 @@ router.route('/')
const { method, params: body } = req.body

if (!_.includes(['create', 'fetch', 'list', 'config', 'remove', 'heartbeat', 'listProjectTerminalShortcuts'], method)) {
throw new Error(`${method} not allowed for terminals`)
throw new UnprocessableEntity(`${method} not allowed for terminals`)
}
res.send(await terminals[method]({ user, body }))
} catch (err) {
Expand Down
39 changes: 36 additions & 3 deletions backend/lib/services/members/MemberManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ class MemberManager {
return this.subjectList.members
}

async rotateServiceAccountSecret (id) {
const item = this.subjectList.get(id)
if (!item) {
return
}

if (item.kind !== 'ServiceAccount') {
throw new UnprocessableEntity('Member is not a ServiceAccount')
}

await this.deleteServiceAccountSecret(item)
}

setItemRoles (item, roles) {
roles = _.compact(roles)
if (!roles.length && item.kind !== 'ServiceAccount') {
Expand All @@ -114,7 +127,7 @@ class MemberManager {
async createServiceAccount (item, { createdBy, description }) {
const { namespace, name } = Member.parseUsername(item.id)
if (namespace !== this.namespace) {
return
return // foreign service account => early exit, nothing to create
}

const serviceAccount = await this.client.core.serviceaccounts.create(namespace, {
Expand Down Expand Up @@ -144,7 +157,7 @@ class MemberManager {
async updateServiceAccount (item, { description }) {
const { namespace, name } = Member.parseUsername(item.id)
if (namespace !== this.namespace) {
return
throw new UnprocessableEntity('It is not possible to modify ServiceAccount from another namespace')
}

const isDirty = item.extend({ description })
Expand All @@ -162,13 +175,33 @@ class MemberManager {
async deleteServiceAccount (item) {
const { namespace, name } = Member.parseUsername(item.id)
if (namespace !== this.namespace) {
return
return // foreign service account => early exit, nothing to delete
}

await this.client.core.serviceaccounts.delete(namespace, name)
this.subjectList.delete(item.id)
}

async deleteServiceAccountSecret (item) {
const { namespace } = Member.parseUsername(item.id)
if (namespace !== this.namespace) {
throw new UnprocessableEntity('It is not possible to modify a ServiceAccount from another namespace')
}

const name = _
.chain(item)
.get('extensions.secrets', [])
.tap(secrets => {
if (secrets.length > 1) {
throw new UnprocessableEntity(`ServiceAccount ${namespace} has more than one secret`)
}
})
.head()
grolu marked this conversation as resolved.
Show resolved Hide resolved
.get('name')
.value()
return await this.client.core.secrets.delete(namespace, name)
grolu marked this conversation as resolved.
Show resolved Hide resolved
}

async getKubeconfig (item) {
const { namespace, name } = Member.parseUsername(item.id)
const secretName = _
Expand Down
5 changes: 5 additions & 0 deletions backend/lib/services/members/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ exports.remove = async function ({ user, namespace, name }) {
const memberManager = await MemberManager.create(user, namespace)
return memberManager.delete(name)
}

exports.rotateSecret = async function ({ user, namespace, name }) {
const memberManager = await MemberManager.create(user, namespace)
memberManager.rotateServiceAccountSecret(name)
grolu marked this conversation as resolved.
Show resolved Hide resolved
}
34 changes: 34 additions & 0 deletions backend/test/acceptance/__snapshots__/api.members.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,40 @@ Array [
]
`;

exports[`api members should rotate a service account secret 1`] = `
Array [
Array [
Object {
":authority": "kubernetes:6443",
":method": "get",
":path": "/apis/core.gardener.cloud/v1beta1/projects/foo",
":scheme": "https",
"authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImJhckBleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.7WKy0sNVkJzIqh3QJIF1zk3QjzwFe_zMTv8PmnOCsxg",
},
],
Array [
Object {
":authority": "kubernetes:6443",
":method": "get",
":path": "/api/v1/namespaces/garden-foo/serviceaccounts",
":scheme": "https",
"authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImJhckBleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.7WKy0sNVkJzIqh3QJIF1zk3QjzwFe_zMTv8PmnOCsxg",
},
],
Array [
Object {
":authority": "kubernetes:6443",
":method": "delete",
":path": "/api/v1/namespaces/garden-foo/secrets/robot-token-726f6",
":scheme": "https",
"authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImJhckBleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.7WKy0sNVkJzIqh3QJIF1zk3QjzwFe_zMTv8PmnOCsxg",
},
],
]
`;

exports[`api members should rotate a service account secret 2`] = `Object {}`;

exports[`api members should update roles of a project member 1`] = `
Array [
Array [
Expand Down
20 changes: 20 additions & 0 deletions backend/test/acceptance/api.members.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,5 +393,25 @@ describe('api', function () {

expect(res.body).toMatchSnapshot()
})

it('should rotate a service account secret', async function () {
const name = 'system:serviceaccount:garden-foo:robot'

mockRequest.mockImplementationOnce(fixtures.projects.mocks.get())
mockRequest.mockImplementationOnce(fixtures.serviceaccounts.mocks.list())
mockRequest.mockImplementationOnce(fixtures.serviceaccounts.mocks.delete())
grolu marked this conversation as resolved.
Show resolved Hide resolved

const res = await agent
.post(`/api/namespaces/${namespace}/members/${name}`)
.set('cookie', await user.cookie)
.send({
method: 'rotateSecret'
})
grolu marked this conversation as resolved.
Show resolved Hide resolved

expect(mockRequest).toBeCalledTimes(3)
expect(mockRequest.mock.calls).toMatchSnapshot()

expect(res.body).toMatchSnapshot()
grolu marked this conversation as resolved.
Show resolved Hide resolved
})
})
})
53 changes: 48 additions & 5 deletions backend/test/services.members.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ describe('services', function () {
'dashboard.gardener.cloud/description': 'description'
},
creationTimestamp: 'bar-time'
}
},
secrets: [
{
name: 'secret-1'
}
]
},
{
metadata: {
Expand All @@ -131,7 +136,15 @@ describe('services', function () {
'dashboard.gardener.cloud/created-by': 'foo'
},
creationTimestamp: 'bar-time'
}
},
secrets: [
{
name: 'secret-1'
},
{
name: 'secret-2'
}
]
},
{
metadata: {
Expand Down Expand Up @@ -171,6 +184,9 @@ describe('services', function () {
delete: jest.fn().mockResolvedValue(),
mergePatch: jest.fn().mockResolvedValue()
}
client.core.secrets = {
delete: jest.fn().mockResolvedValue()
}
})

describe('SubjectList', function () {
Expand Down Expand Up @@ -361,6 +377,13 @@ describe('services', function () {
})

describe('#deleteServiceAccount', function () {
it('should delete a serviceaccount', async function () {
const id = 'system:serviceaccount:garden-foo:robot-sa'
const item = memberManager.subjectList.get(id)
await memberManager.deleteServiceAccount(item)
expect(client.core.serviceaccounts.delete).toBeCalledWith('garden-foo', 'robot-sa')
})

it('should not delete a serviceaccount from a different namespace', async function () {
const id = 'system:serviceaccount:garden-foreign:robot-foreign-namespace'
const item = memberManager.subjectList.get(id)
Expand All @@ -370,11 +393,31 @@ describe('services', function () {
})

describe('#updateServiceAccount ', function () {
it('should not delete a serviceaccount from a different namespace', async function () {
it('should not update a serviceaccount from a different namespace', async function () {
const id = 'system:serviceaccount:garden-foreign:robot-foreign-namespace'
const item = memberManager.subjectList.get(id)
await memberManager.updateServiceAccount(item, {})
expect(client.core.serviceaccounts.mergePatch).not.toBeCalled()
await expect(memberManager.updateServiceAccount(item, {})).rejects.toThrow(UnprocessableEntity)
})
})

describe('#deleteServiceAccountSecret', function () {
it('should delete a serviceaccount secret', async function () {
const id = 'system:serviceaccount:garden-foo:robot-sa'
const item = memberManager.subjectList.get(id)
await memberManager.deleteServiceAccountSecret(item)
expect(client.core.secrets.delete).toBeCalledWith('garden-foo', 'secret-1')
})

it('should not delete a serviceaccount secret from a different namespace', async function () {
const id = 'system:serviceaccount:garden-foreign:robot-foreign-namespace'
const item = memberManager.subjectList.get(id)
await expect(memberManager.deleteServiceAccountSecret(item)).rejects.toThrow(UnprocessableEntity)
})

it('should not delete a service account secret if there is one more secret attached', async function () {
const id = 'system:serviceaccount:garden-foo:robot-multiple'
const item = memberManager.subjectList.get(id)
await expect(memberManager.deleteServiceAccountSecret(item)).rejects.toThrow(UnprocessableEntity)
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ SPDX-License-Identifier: Apache-2.0
-->

<template>
<div d-flex flex-row>
<v-tooltip top v-for="{ displayName, notEditable, tooltip } in roleDisplayNames" :key="displayName" :disabled="!tooltip">
<div>
<v-tooltip top v-for="({ displayName, notEditable, tooltip }, index) in roleDisplayNames" :key="displayName" :disabled="!tooltip">
<template v-slot:activator="{ on }">
<v-chip v-on="on" class="mr-3" small :color="notEditable ? 'grey' : 'black'" outlined>
<v-chip v-on="on" small :color="notEditable ? 'grey' : 'black'" outlined :class="{'ml-3': index > 0}">
{{displayName}}
</v-chip>
</template>
Expand Down
Loading