Skip to content

Commit

Permalink
fix(api): edit as code worker model with a non-admin user (#4097)
Browse files Browse the repository at this point in the history
Signed-off-by: Benjamin Coenen <[email protected]>
  • Loading branch information
bnjjj authored and yesnault committed Apr 1, 2019
1 parent 75cf5b6 commit a10545f
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 36 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
.vscode
.DS_Store
dump.rdb
dist
cli/cds/cds
engine/engine
engine/dist
Expand Down
2 changes: 1 addition & 1 deletion engine/api/api_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func (api *API) InitRouter() {
r.Handle("/worker/model/type", r.GET(api.getWorkerModelTypesHandler))
r.Handle("/worker/model/communication", r.GET(api.getWorkerModelCommunicationsHandler))
r.Handle("/worker/model/{permModelID}", r.PUT(api.updateWorkerModelHandler), r.DELETE(api.deleteWorkerModelHandler))
r.Handle("/worker/model/{permModelID}/export", r.GET(api.getWorkerModelExportHandler))
r.Handle("/worker/model/{modelID}/export", r.GET(api.getWorkerModelExportHandler))
r.Handle("/worker/model/{modelID}/usage", r.GET(api.getWorkerModelUsageHandler))
r.Handle("/worker/model/capability/type", r.GET(api.getRequirementTypesHandler))

Expand Down
2 changes: 1 addition & 1 deletion engine/api/worker/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func updateAllToCheckRegistration(db gorp.SqlExecutor) error {

// UpdateSpawnErrorWorkerModel updates worker model error registration
func UpdateSpawnErrorWorkerModel(db gorp.SqlExecutor, modelID int64, spawnError sdk.SpawnErrorForm) error {
// some times when the docker container fails to start, the docker logs is not empty but only contains utf8 null char
// some times when the docker container fails to start, the docker logs is not empty but only contains utf8 null char
if spawnError.Error == string([]byte{0x00}) {
spawnError.Error = ""
}
Expand Down
4 changes: 2 additions & 2 deletions engine/api/worker/model_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
)

// Export convert sdk.Model to an exportentities.WorkerModel, format and write into a io.Writer
func Export(wm sdk.Model, f exportentities.Format, w io.Writer) (int, error) {
eWm := exportentities.NewWorkerModel(wm)
func Export(wm sdk.Model, f exportentities.Format, w io.Writer, opts ...exportentities.WorkerModelOption) (int, error) {
eWm := exportentities.NewWorkerModel(wm, opts...)

// Marshal to the desired format
b, err := exportentities.Marshal(eWm, f)
Expand Down
68 changes: 50 additions & 18 deletions engine/api/worker/model_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import (

// ParseAndImport parse and import an exportentities.WorkerModel
func ParseAndImport(db gorp.SqlExecutor, store cache.Store, eWorkerModel *exportentities.WorkerModel, force bool, u *sdk.User) (*sdk.Model, error) {
sdkWm, err := eWorkerModel.GetWorkerModel()
if err != nil {
return nil, err
}

sdkWm, errInvalidModel := eWorkerModel.GetWorkerModel()
gr, err := group.LoadGroupByName(db, sdkWm.Group.Name)
if err != nil {
return nil, sdk.WrapError(err, "Unable to get group %s", sdkWm.Group.Name)
Expand Down Expand Up @@ -55,41 +51,33 @@ currentUGroup:
return nil, sdk.ErrWorkerModelNoAdmin
}

var badRequestError error
asSimpleUser := !u.Admin && !sdkWm.Restricted
switch sdkWm.Type {
case sdk.Docker:
if sdkWm.ModelDocker.Image == "" {
return nil, sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker image")
}
if !u.Admin && !sdkWm.Restricted {
if modelPattern == nil {
return nil, sdk.ErrWorkerModelNoPattern
}
}
if modelPattern != nil {
sdkWm.ModelDocker.Cmd = modelPattern.Model.Cmd
sdkWm.ModelDocker.Shell = modelPattern.Model.Shell
sdkWm.ModelDocker.Envs = modelPattern.Model.Envs
}
if sdkWm.ModelDocker.Cmd == "" || sdkWm.ModelDocker.Shell == "" {
return nil, sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker command or invalid shell command")
badRequestError = sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker command or invalid shell command")
}
default:
if sdkWm.ModelVirtualMachine.Image == "" {
return nil, sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker image: cannot be empty")
}
if !u.Admin && !sdkWm.Restricted {
if modelPattern == nil {
return nil, sdk.ErrWorkerModelNoPattern
}
}
if modelPattern != nil {
sdkWm.ModelVirtualMachine.PreCmd = modelPattern.Model.PreCmd
sdkWm.ModelVirtualMachine.Cmd = modelPattern.Model.Cmd
sdkWm.ModelVirtualMachine.PostCmd = modelPattern.Model.PostCmd
}

if sdkWm.ModelVirtualMachine.Cmd == "" {
return nil, sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker command: Cannot be empty")
badRequestError = sdk.NewErrorFrom(sdk.ErrWrongRequest, "Invalid worker command: Cannot be empty")
}
}

Expand All @@ -103,13 +91,22 @@ currentUGroup:

// provision is allowed only for CDS Admin
// or by currentUser with a restricted model
if !u.Admin && !sdkWm.Restricted {
if asSimpleUser {
sdkWm.Provision = 0
}

if force {
if existingWm, err := LoadWorkerModelByName(db, sdkWm.Name); err != nil {
if sdk.Cause(err) == sql.ErrNoRows {
if asSimpleUser && modelPattern == nil {
return nil, sdk.ErrWorkerModelNoPattern
}
if errInvalidModel != nil {
return nil, errInvalidModel
}
if badRequestError != nil {
return nil, badRequestError
}
if errAdd := InsertWorkerModel(db, &sdkWm); errAdd != nil {
return nil, sdk.WrapError(errAdd, "cannot add worker model %s", sdkWm.Name)
}
Expand All @@ -118,13 +115,48 @@ currentUGroup:
}
} else {
sdkWm.ID = existingWm.ID
if asSimpleUser && modelPattern == nil {
if existingWm.Type != sdk.Docker || existingWm.Restricted != sdkWm.Restricted { // Forbidden because we can't fetch previous user data
return nil, sdk.WrapError(sdk.ErrWorkerModelNoPattern, "we can't fetch previous user data because type or restricted is different")
}
switch sdkWm.Type {
case sdk.Docker:
img := sdkWm.ModelDocker.Image
sdkWm.ModelDocker = existingWm.ModelDocker
sdkWm.ModelDocker.Image = img
default:
img := sdkWm.ModelVirtualMachine.Image
flavor := sdkWm.ModelVirtualMachine.Flavor
sdkWm.ModelVirtualMachine = existingWm.ModelVirtualMachine
sdkWm.ModelVirtualMachine.Image = img
sdkWm.ModelVirtualMachine.Flavor = flavor
}
}
if !asSimpleUser {
if errInvalidModel != nil {
return nil, errInvalidModel
}
if badRequestError != nil {
return nil, badRequestError
}
}

if errU := UpdateWorkerModel(db, &sdkWm); errU != nil {
return nil, sdk.WrapError(errU, "cannot update worker model %s", sdkWm.Name)
}
}
return &sdkWm, nil
}

if asSimpleUser && modelPattern == nil {
return nil, sdk.ErrWorkerModelNoPattern
}
if errInvalidModel != nil {
return nil, errInvalidModel
}
if badRequestError != nil {
return nil, badRequestError
}
if errAdd := InsertWorkerModel(db, &sdkWm); errAdd != nil {
if errPG, ok := sdk.Cause(errAdd).(*pq.Error); ok && errPG.Code == gorpmapping.ViolateUniqueKeyPGCode {
errAdd = sdk.ErrConflict
Expand Down
10 changes: 5 additions & 5 deletions engine/api/worker_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ func (api *API) updateWorkerModelHandler() service.Handler {
//User must be admin of the group set in the model
var ok bool
currentUGroup:
for _, g := range deprecatedGetUser(ctx).Groups {
for _, g := range user.Groups {
if g.ID == model.GroupID {
for _, a := range g.Admins {
if a.ID == deprecatedGetUser(ctx).ID {
if a.ID == user.ID {
ok = true
break currentUGroup
}
Expand All @@ -276,7 +276,7 @@ func (api *API) updateWorkerModelHandler() service.Handler {
}

//User should have the right permission or be admin
if !deprecatedGetUser(ctx).Admin && !ok {
if !user.Admin && !ok {
return sdk.ErrWorkerModelNoAdmin
}

Expand All @@ -287,8 +287,8 @@ func (api *API) updateWorkerModelHandler() service.Handler {
}
if !user.Admin && !model.Restricted {
if modelPattern == nil {
if old.Type != sdk.Docker { // Forbidden because we can't fetch previous user data
return sdk.WrapError(sdk.ErrWorkerModelNoPattern, "we can't fetch previous user data because type is different")
if old.Type != sdk.Docker || old.Restricted != model.Restricted { // Forbidden because we can't fetch previous user data
return sdk.WrapError(sdk.ErrWorkerModelNoPattern, "we can't fetch previous user data because type or restricted is different")
}
model.ModelDocker.Cmd = old.ModelDocker.Cmd
model.ModelDocker.Shell = old.ModelDocker.Shell
Expand Down
12 changes: 9 additions & 3 deletions engine/api/worker_model_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (

func (api *API) getWorkerModelExportHandler() service.Handler {
return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
workerModelID, errr := requestVarInt(r, "permModelID")
u := deprecatedGetUser(ctx)
workerModelID, errr := requestVarInt(r, "modelID")
if errr != nil {
return sdk.WrapError(errr, "Invalid permModelID")
return sdk.WrapError(errr, "Invalid modelID")
}

format := FormString(r, "format")
Expand All @@ -33,7 +34,12 @@ func (api *API) getWorkerModelExportHandler() service.Handler {
return sdk.WrapError(err, "Format invalid")
}

if _, err := worker.Export(*wm, f, w); err != nil {
opts := []exportentities.WorkerModelOption{}
if u != nil && !u.Admin && !wm.Restricted {
opts = append(opts, exportentities.WorkerModelLoadOptions.HideAdminFields)
}

if _, err := worker.Export(*wm, f, w, opts...); err != nil {
return err
}

Expand Down
29 changes: 23 additions & 6 deletions sdk/exportentities/worker_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,25 @@ type WorkerModel struct {
IsDeprecated bool `json:"is_deprecated,omitempty" yaml:"is_deprecated,omitempty"`
}

type WorkerModelOption func(sdk.Model, *WorkerModel) error

var WorkerModelLoadOptions = struct {
HideAdminFields WorkerModelOption
}{
HideAdminFields: loadWorkerModelWithoutAdminFields,
}

func loadWorkerModelWithoutAdminFields(_ sdk.Model, wm *WorkerModel) error {
wm.PreCmd = ""
wm.Shell = ""
wm.Cmd = ""
wm.PostCmd = ""
wm.Envs = nil
return nil
}

// NewWorkerModel creates an exportentities WorkerModel from a struct sdk.Model
func NewWorkerModel(wm sdk.Model) WorkerModel {
func NewWorkerModel(wm sdk.Model, opts ...WorkerModelOption) WorkerModel {
model := WorkerModel{
Type: wm.Type,
Name: wm.Name,
Expand Down Expand Up @@ -53,15 +70,15 @@ func NewWorkerModel(wm sdk.Model) WorkerModel {
model.PostCmd = wm.ModelVirtualMachine.PostCmd
}

for _, opt := range opts {
_ = opt(wm, &model)
}

return model
}

// GetWorkerModel convert an exportentities to a real sdk.Model
func (wm WorkerModel) GetWorkerModel() (sdk.Model, error) {
if err := wm.IsValid(); err != nil {
return sdk.Model{}, err
}

model := sdk.Model{
Type: wm.Type,
Name: wm.Name,
Expand Down Expand Up @@ -92,7 +109,7 @@ func (wm WorkerModel) GetWorkerModel() (sdk.Model, error) {
}
}

return model, nil
return model, wm.IsValid()
}

// IsValid returns error if worker model is invalid
Expand Down

0 comments on commit a10545f

Please sign in to comment.