Skip to content

Commit

Permalink
Do not return consolepassword in machine describe anymore (#188)
Browse files Browse the repository at this point in the history
  • Loading branch information
majst01 authored Jun 7, 2021
1 parent dad987e commit a82f2b4
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 77 deletions.
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ MINI_LAB_KUBECONFIG := $(shell pwd)/../mini-lab/.kubeconfig

include $(COMMONDIR)/Makefile.inc

release:: protoc spec all ;
release:: protoc spec check-diff all ;

.PHONY: spec
spec: all
bin/$(BINARY) dump-swagger | jq -r -S 'walk(if type == "array" then sort_by(strings) else . end)' > spec/metal-api.json || { echo "jq >=1.6 required"; exit 1; }

.PHONY: check-diff
check-diff: spec
git diff --exit-code spec pkg

.PHONY: redoc
redoc:
docker run --rm --user $$(id -u):$$(id -g) -v $(PWD):/work -w /work letsdeal/redoc-cli bundle -o generate/index.html /work/spec/metal-api.json
Expand All @@ -22,6 +26,7 @@ protoc:

.PHONY: protoc-docker
protoc-docker:
docker pull metalstack/builder
docker run --rm --user $$(id -u):$$(id -g) -v $(PWD):/work -w /work metalstack/builder protoc -I pkg --go_out plugins=grpc:pkg pkg/api/v1/*.proto

.PHONY: mini-lab-push
Expand Down
2 changes: 1 addition & 1 deletion cmd/metal-api/internal/service/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func createTestEnvironment(t *testing.T) testEnv {
}()
grpcServer.Publisher = NopPublisher{} // has to be done after constructor because init would fail otherwise

machineService, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipamer, mdc, grpcServer, nil)
machineService, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipamer, mdc, grpcServer, nil, nil, 0)
require.NoError(err)
imageService := NewImage(ds)
switchService := NewSwitch(ds)
Expand Down
81 changes: 70 additions & 11 deletions cmd/metal-api/internal/service/machine-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/avast/retry-go"
s3server "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/s3client"
"github.com/metal-stack/security"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/grpc"
"github.com/pkg/errors"
Expand Down Expand Up @@ -41,11 +42,13 @@ import (
type machineResource struct {
webResource
bus.Publisher
ipamer ipam.IPAMer
mdc mdm.Client
actor *asyncActor
grpcServer *grpc.Server
s3Client *s3server.Client
ipamer ipam.IPAMer
mdc mdm.Client
actor *asyncActor
grpcServer *grpc.Server
s3Client *s3server.Client
userGetter security.UserGetter
reasonMinLength uint
}

// machineAllocationSpec is a specification for a machine allocation
Expand Down Expand Up @@ -101,17 +104,21 @@ func NewMachine(
ipamer ipam.IPAMer,
mdc mdm.Client,
grpcServer *grpc.Server,
s3Client *s3server.Client) (*restful.WebService, error) {
s3Client *s3server.Client,
userGetter security.UserGetter,
reasonMinLength uint) (*restful.WebService, error) {

r := machineResource{
webResource: webResource{
ds: ds,
},
Publisher: pub,
ipamer: ipamer,
mdc: mdc,
grpcServer: grpcServer,
s3Client: s3Client,
Publisher: pub,
ipamer: ipamer,
mdc: mdc,
grpcServer: grpcServer,
s3Client: s3Client,
userGetter: userGetter,
reasonMinLength: reasonMinLength,
}
var err error
r.actor, err = newAsyncActor(zapup.MustRootLogger(), ep, ds, ipamer)
Expand Down Expand Up @@ -142,6 +149,16 @@ func (r machineResource) webService() *restful.WebService {
Returns(http.StatusOK, "OK", v1.MachineResponse{}).
DefaultReturns("Error", httperrors.HTTPErrorResponse{}))

ws.Route(ws.GET("/consolepassword").
To(editor(r.getMachineConsolePassword)).
Operation("getMachineConsolePassword").
Doc("get consolepassword for machine by id").
Reads(v1.MachineConsolePasswordRequest{}).
Metadata(restfulspec.KeyOpenAPITags, tags).
Writes(v1.MachineConsolePasswordResponse{}).
Returns(http.StatusOK, "OK", v1.MachineConsolePasswordResponse{}).
DefaultReturns("Error", httperrors.HTTPErrorResponse{}))

ws.Route(ws.GET("/").
To(viewer(r.listMachines)).
Operation("listMachines").
Expand Down Expand Up @@ -440,6 +457,48 @@ func (r machineResource) findMachine(request *restful.Request, response *restful
}
}

func (r machineResource) getMachineConsolePassword(request *restful.Request, response *restful.Response) {
var requestPayload v1.MachineConsolePasswordRequest
err := request.ReadEntity(&requestPayload)
if checkError(request, response, utils.CurrentFuncName(), err) {
return
}

if uint(len(requestPayload.Reason)) < r.reasonMinLength {
if checkError(request, response, utils.CurrentFuncName(), fmt.Errorf("reason must be at least %d characters long", r.reasonMinLength)) {
return
}
}
user, err := r.userGetter.User(request.Request)
if checkError(request, response, utils.CurrentFuncName(), err) {
return
}

m, err := r.ds.FindMachineByID(requestPayload.ID)
if checkError(request, response, utils.CurrentFuncName(), err) {
return
}

if m.Allocation == nil {
if checkError(request, response, utils.CurrentFuncName(), fmt.Errorf("machine is not allocated, no consolepassword present")) {
return
}
}

resp := v1.MachineConsolePasswordResponse{
Common: v1.Common{Identifiable: v1.Identifiable{ID: m.ID}, Describable: v1.Describable{Name: &m.Name, Description: &m.Description}},
ConsolePassword: m.Allocation.ConsolePassword,
}

zapup.MustRootLogger().Sugar().Infow("consolepassword requested", "machine", m.ID, "user", user.Name, "email", user.EMail, "tenant", user.Tenant, "reason", requestPayload.Reason)

err = response.WriteHeaderAndEntity(http.StatusOK, resp)
if err != nil {
zapup.MustRootLogger().Error("Failed to send response", zap.Error(err))
return
}
}

func (r machineResource) findMachines(request *restful.Request, response *restful.Response) {
var requestPayload datastore.MachineSearchQuery
err := request.ReadEntity(&requestPayload)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func setupTestEnvironment(machineCount int, t *testing.T) (*datastore.RethinkSto

createTestdata(machineCount, rs, ipamer, t)

ms, err := NewMachine(rs, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(ipamer), mdc, ws, nil)
ms, err := NewMachine(rs, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(ipamer), mdc, ws, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(ms)
usergetter := security.NewCreds(security.WithHMAC(hma))
Expand Down
27 changes: 13 additions & 14 deletions cmd/metal-api/internal/service/machine-service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestGetMachines(t *testing.T) {
ds, mock := datastore.InitMockDB()
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(machineservice)
req := httptest.NewRequest("GET", "/v1/machine", nil)
Expand Down Expand Up @@ -226,7 +226,7 @@ func TestRegisterMachine(t *testing.T) {

js, _ := json.Marshal(registerRequest)
body := bytes.NewBuffer(js)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(machineservice)
req := httptest.NewRequest("POST", "/v1/machine/register", body)
Expand Down Expand Up @@ -302,7 +302,7 @@ func TestMachineIPMIReport(t *testing.T) {
for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(machineservice)
js, _ := json.Marshal(tt.input)
Expand Down Expand Up @@ -350,7 +350,7 @@ func TestMachineFindIPMI(t *testing.T) {
mock.On(r.DB("mockdb").Table("machine").Filter(r.MockAnything())).Return([]interface{}{*tt.machine}, nil)
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(machineservice)

Expand Down Expand Up @@ -429,12 +429,12 @@ func TestFinalizeMachineAllocation(t *testing.T) {
for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)
container := restful.NewContainer().Add(machineservice)

finalizeRequest := v1.MachineFinalizeAllocationRequest{
ConsolePassword: "blubber",
Kernel: "vmlinuz",
}

js, _ := json.Marshal(finalizeRequest)
Expand Down Expand Up @@ -463,7 +463,6 @@ func TestFinalizeMachineAllocation(t *testing.T) {
err := json.NewDecoder(resp.Body).Decode(&result)

require.Nil(t, err)
require.Equal(t, finalizeRequest.ConsolePassword, *result.Allocation.ConsolePassword)
}
})
}
Expand All @@ -473,7 +472,7 @@ func TestSetMachineState(t *testing.T) {
ds, mock := datastore.InitMockDB()
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand Down Expand Up @@ -506,7 +505,7 @@ func TestGetMachine(t *testing.T) {
ds, mock := datastore.InitMockDB()
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand All @@ -533,7 +532,7 @@ func TestGetMachineNotFound(t *testing.T) {
ds, mock := datastore.InitMockDB()
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand Down Expand Up @@ -566,7 +565,7 @@ func TestFreeMachine(t *testing.T) {
return nil
}

machineservice, err := NewMachine(ds, pub, bus.NewEndpoints(nil, pub), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, pub, bus.NewEndpoints(nil, pub), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand All @@ -592,7 +591,7 @@ func TestSearchMachine(t *testing.T) {
mock.On(r.DB("mockdb").Table("machine").Filter(r.MockAnything())).Return([]interface{}{testdata.M1}, nil)
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand Down Expand Up @@ -623,7 +622,7 @@ func TestAddProvisioningEvent(t *testing.T) {
ds, mock := datastore.InitMockDB()
testdata.InitMockDBData(mock)

machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, &emptyPublisher{}, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

container := restful.NewContainer().Add(machineservice)
Expand Down Expand Up @@ -713,7 +712,7 @@ func TestOnMachine(t *testing.T) {
return nil
}

machineservice, err := NewMachine(ds, pub, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil)
machineservice, err := NewMachine(ds, pub, bus.DirectEndpoints(), ipam.New(goipam.New()), nil, nil, nil, nil, 0)
require.NoError(t, err)

js, _ := json.Marshal([]string{tt.param})
Expand Down
16 changes: 9 additions & 7 deletions cmd/metal-api/internal/service/v1/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ type MachineAllocation struct {
Hostname string `json:"hostname" description:"the hostname which will be used when creating the machine"`
SSHPubKeys []string `json:"ssh_pub_keys" description:"the public ssh keys to access the machine with"`
UserData string `json:"user_data,omitempty" description:"userdata to execute post installation tasks" optional:"true"`
ConsolePassword *string `json:"console_password" description:"the console password which was generated while provisioning" optional:"true"`
Succeeded bool `json:"succeeded" description:"if the allocation of the machine was successful, this is set to true"`
Reinstall bool `json:"reinstall" description:"indicates whether to reinstall the machine"`
BootInfo *BootInfo `json:"boot_info" description:"information required for booting the machine from HD" optional:"true"`
Expand Down Expand Up @@ -224,6 +223,15 @@ type MachineResponse struct {
Timestamps
}

type MachineConsolePasswordRequest struct {
ID string `json:"id" description:"id of the machine to get the consolepassword for"`
Reason string `json:"reason" description:"reason why the consolepassword is requested, typically a incident number with short description"`
}
type MachineConsolePasswordResponse struct {
Common
ConsolePassword string `json:"console_password" description:"the console password which was generated while provisioning"`
}

type MachineIPMIResponse struct {
Common
MachineBase
Expand Down Expand Up @@ -443,11 +451,6 @@ func NewMachineResponse(m *metal.Machine, s *metal.Size, p *metal.Partition, i *
networks = append(networks, network)
}

var consolePassword *string
if m.Allocation.ConsolePassword != "" {
consolePassword = &m.Allocation.ConsolePassword
}

allocation = &MachineAllocation{
Created: m.Allocation.Created,
Name: m.Allocation.Name,
Expand All @@ -457,7 +460,6 @@ func NewMachineResponse(m *metal.Machine, s *metal.Size, p *metal.Partition, i *
Hostname: m.Allocation.Hostname,
SSHPubKeys: m.Allocation.SSHPubKeys,
UserData: m.Allocation.UserData,
ConsolePassword: consolePassword,
MachineNetworks: networks,
Succeeded: m.Allocation.Succeeded,
}
Expand Down
18 changes: 13 additions & 5 deletions cmd/metal-api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
v1 "github.com/metal-stack/masterdata-api/api/v1"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/service/s3client"
"google.golang.org/protobuf/types/known/wrapperspb"
"net/http"
httppprof "net/http/pprof"
"os"
Expand All @@ -15,6 +12,10 @@ import (
"syscall"
"time"

v1 "github.com/metal-stack/masterdata-api/api/v1"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/service/s3client"
"google.golang.org/protobuf/types/known/wrapperspb"

"github.com/go-logr/zapr"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/grpc"
Expand Down Expand Up @@ -199,6 +200,7 @@ func init() {
rootCmd.Flags().IntP("port", "", 8080, "the port to serve on")
rootCmd.Flags().IntP("grpc-port", "", 50051, "the port to serve gRPC on")
rootCmd.Flags().Bool("init-data-store", true, "initializes the data store on start (can be switched off when running the init command before starting instances)")
rootCmd.Flags().UintP("password-reason-minlength", "", 0, "if machine console password is requested this defines if and how long the given reason must be")

rootCmd.Flags().StringP("base-path", "", "/", "the base path of the api server")

Expand Down Expand Up @@ -684,7 +686,13 @@ func initRestServices(withauth bool) *restfulspec.Config {
if err != nil {
logger.Fatal(err)
}
machineService, err := service.NewMachine(ds, p, ep, ipamer, mdc, grpcServer, s3Client)
var userGetter security.UserGetter
if withauth {
userGetter = initAuth(lg.Sugar())
}
reasonMinLength := viper.GetUint("password-reason-minlength")

machineService, err := service.NewMachine(ds, p, ep, ipamer, mdc, grpcServer, s3Client, userGetter, reasonMinLength)
if err != nil {
logger.Fatal(err)
}
Expand All @@ -710,7 +718,7 @@ func initRestServices(withauth bool) *restfulspec.Config {
restful.DefaultContainer.Filter(metrics.RestfulMetrics)

if withauth {
restful.DefaultContainer.Filter(rest.UserAuth(initAuth(lg.Sugar())))
restful.DefaultContainer.Filter(rest.UserAuth(userGetter))
providerTenant := viper.GetString("provider-tenant")
excludedPathSuffixes := []string{"liveliness", "health", "version", "apidocs.json"}
ensurer := service.NewTenantEnsurer([]string{providerTenant}, excludedPathSuffixes)
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ require (
github.com/go-logr/zapr v0.4.0
github.com/go-openapi/spec v0.19.15
github.com/go-stack/stack v1.8.0
github.com/golang/protobuf v1.4.3
github.com/google/go-cmp v0.5.5
github.com/google/uuid v1.2.0
github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 // indirect
Expand Down
Loading

0 comments on commit a82f2b4

Please sign in to comment.