Skip to content

Commit

Permalink
fix(api): return not found if project not readable, marathon cpu in c…
Browse files Browse the repository at this point in the history
…onf (#4899)
  • Loading branch information
richardlt committed Jan 20, 2020
1 parent 28f7ba1 commit f561705
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 24 deletions.
24 changes: 12 additions & 12 deletions cli/cdsctl/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ func projectDeleteRun(v cli.Values) error {
projKey := v.GetString(_ProjectKey)
if v.GetBool("force") {
// Delete all workflow
ws, errW := client.WorkflowList(projKey)
if errW != nil && !sdk.ErrorIs(errW, sdk.ErrNoProject) {
return errW
ws, err := client.WorkflowList(projKey)
if err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
return err
}
for _, w := range ws {
if err := client.WorkflowDelete(projKey, w.Name); err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
Expand All @@ -169,9 +169,9 @@ func projectDeleteRun(v cli.Values) error {
}

// Delete all apps
apps, errA := client.ApplicationList(projKey)
if errA != nil && !sdk.ErrorIs(errA, sdk.ErrNoProject) {
return errA
apps, err := client.ApplicationList(projKey)
if err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
return err
}
for _, app := range apps {
if err := client.ApplicationDelete(projKey, app.Name); err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
Expand All @@ -180,9 +180,9 @@ func projectDeleteRun(v cli.Values) error {
}

// Delete all pipelines
pips, errP := client.PipelineList(projKey)
if errP != nil && !sdk.ErrorIs(errP, sdk.ErrNoProject) {
return errP
pips, err := client.PipelineList(projKey)
if err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
return err
}
for _, pip := range pips {
if err := client.PipelineDelete(projKey, pip.Name); err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
Expand All @@ -191,9 +191,9 @@ func projectDeleteRun(v cli.Values) error {
}

// Delete all environments
envs, errE := client.EnvironmentList(projKey)
if errE != nil && !sdk.ErrorIs(errE, sdk.ErrNoProject) {
return errE
envs, err := client.EnvironmentList(projKey)
if err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
return err
}
for _, env := range envs {
if err := client.EnvironmentDelete(projKey, env.Name); err != nil && !sdk.ErrorIs(err, sdk.ErrNoProject) {
Expand Down
4 changes: 3 additions & 1 deletion engine/api/ascode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func Test_getImportAsCodeHandler(t *testing.T) {
api, db, _, end := newTestAPI(t)
defer end()

p := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10))
u, pass := assets.InsertAdminUser(t, db)

_, _ = assets.InsertService(t, db, "Test_getImportAsCodeHandler", services.TypeRepositories)
Expand Down Expand Up @@ -164,7 +165,8 @@ func Test_getImportAsCodeHandler(t *testing.T) {
)

uri := api.Router.GetRoute("GET", api.getImportAsCodeHandler, map[string]string{
"uuid": UUID,
"permProjectKey": p.Key,
"uuid": UUID,
})
req, err := http.NewRequest("GET", uri, nil)
test.NoError(t, err)
Expand Down
23 changes: 14 additions & 9 deletions engine/api/router_middleware_auth_permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strconv"

"github.com/ovh/cds/engine/api/authentication"
"github.com/ovh/cds/engine/api/project"
"github.com/ovh/cds/engine/api/cache"
"github.com/ovh/cds/engine/api/observability"
"github.com/ovh/cds/engine/api/workflow"
Expand Down Expand Up @@ -93,24 +94,29 @@ func (api *API) checkJobIDPermissions(ctx context.Context, jobID string, perm in
return nil
}

func (api *API) checkProjectPermissions(ctx context.Context, projectKey string, perm int, routeVars map[string]string) error {
func (api *API) checkProjectPermissions(ctx context.Context, projectKey string, requiredPerm int, routeVars map[string]string) error {
ctx, end := observability.Span(ctx, "api.checkProjectPermissions")
defer end()

if _, err := project.Load(api.mustDB(), api.Cache, projectKey); err != nil {
return err
}

perms, err := permission.LoadProjectMaxLevelPermission(ctx, api.mustDB(), []string{projectKey}, getAPIConsumer(ctx).GetGroupIDs())
if err != nil {
return sdk.NewErrorWithStack(err, sdk.WrapError(sdk.ErrForbidden, "not authorized for project %s", projectKey))
return sdk.WrapError(err, "cannot get max project permissions for %s", projectKey)
}

maxLevelPermission := perms.Level(projectKey)
if maxLevelPermission < perm { // If the caller based on its group doesn have enough permission level
log.Debug("checkProjectPermissions> maxLevelPermission= %d ", maxLevelPermission)
callerPermission := perms.Level(projectKey)
// If the caller based on its group doesn't have enough permission level
if callerPermission < requiredPerm {
log.Debug("checkProjectPermissions> callerPermission=%d ", callerPermission)
// If it's about READ: we have to check if the user is a maintainer or an admin
if perm < sdk.PermissionReadExecute {
if requiredPerm == sdk.PermissionRead {
if !isMaintainer(ctx) {
// The caller doesn't enough permission level from its groups and is neither a maintainer nor an admin
log.Debug("checkProjectPermissions> %s(%s) is not authorized to %s", getAPIConsumer(ctx).Name, getAPIConsumer(ctx).ID, projectKey)
return sdk.WrapError(sdk.ErrForbidden, "not authorized for workflow %s", projectKey)
return sdk.WrapError(sdk.ErrNoProject, "not authorized for project %s", projectKey)
}
log.Debug("checkProjectPermissions> %s(%s) access granted to %s because is maintainer", getAPIConsumer(ctx).Name, getAPIConsumer(ctx).ID, projectKey)
observability.Current(ctx, observability.Tag(observability.TagPermission, "is_maintainer"))
Expand All @@ -126,9 +132,8 @@ func (api *API) checkProjectPermissions(ctx context.Context, projectKey string,
log.Debug("checkProjectPermissions> %s(%s) access granted to %s because is admin", getAPIConsumer(ctx).Name, getAPIConsumer(ctx).ID, projectKey)
observability.Current(ctx, observability.Tag(observability.TagPermission, "is_admin"))
return nil

}
log.Debug("checkWorkflowPermissions> %s(%s) access granted to %s because has permission (max permission = %d)", getAPIConsumer(ctx).Name, getAPIConsumer(ctx).ID, projectKey, maxLevelPermission)
log.Debug("checkWorkflowPermissions> %s(%s) access granted to %s because has permission (max permission = %d)", getAPIConsumer(ctx).Name, getAPIConsumer(ctx).ID, projectKey, callerPermission)
observability.Current(ctx, observability.Tag(observability.TagPermission, "is_granted"))
return nil
}
Expand Down
1 change: 1 addition & 0 deletions engine/hatchery/marathon/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func InitMarathonMarathonTest(opts marathonJDD) *HatcheryMarathon {
h.marathonClient, _ = marathon.NewClient(config)
h.Config.Provision.MaxConcurrentProvisioning = opts.MaxProvision
h.Config.Provision.MaxWorker = opts.MaxWorker
h.Config.DefaultCPUs = 1
if opts.Prefix != "" {
h.Config.MarathonIDPrefix = opts.Prefix
}
Expand Down
2 changes: 1 addition & 1 deletion engine/hatchery/marathon/marathon.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func (h *HatcheryMarathon) SpawnWorker(ctx context.Context, spawnArgs hatchery.S
Type: "DOCKER",
},
Env: &envsWm,
CPUs: 2,
CPUs: h.Config.DefaultCPUs,
Instances: &instance,
Mem: &mem,
Labels: &h.marathonLabels,
Expand Down
2 changes: 1 addition & 1 deletion engine/hatchery/marathon/marathon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func TestSpawnWorkerTimeout(t *testing.T) {
assert.NoError(t, json.Unmarshal(bodyContent, &a))
assert.Equal(t, "DOCKER", a.Container.Type)
assert.Equal(t, "BRIDGE", a.Container.Docker.Network)
assert.Equal(t, float64(2), a.CPUs)
assert.Equal(t, float64(1), a.CPUs)
assert.Equal(t, 1, *a.Instances)
assert.Equal(t, "1", (*a.Env)["CDS_BOOKED_WORKFLOW_JOB_ID"])
assert.Equal(t, "GroupModel/fake", (*a.Env)["CDS_MODEL_PATH"])
Expand Down
3 changes: 3 additions & 0 deletions engine/hatchery/marathon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type HatcheryConfiguration struct {
// MarathonLabelsStr "marathon-labels"
MarathonLabels string `mapstructure:"labels" toml:"labels" default:"" commented:"false" comment:"Use this option if you want to add labels on workers spawned by this hatchery.\n Format: MarathonLabels = \"A_LABEL=value-of-label,B_LABEL=value-of-label-b\"" json:"labels"`

// DefaultCPUs
DefaultCPUs float64 `mapstructure:"defaultCPUs" toml:"defaultCPUs" default:"1" commented:"false" comment:"Worker default CPUs count" json:"defaultCPUs"`

// DefaultMemory Worker default memory
DefaultMemory int `mapstructure:"defaultMemory" toml:"defaultMemory" default:"1024" commented:"false" comment:"Worker default memory in Mo" json:"defaultMemory"`

Expand Down

0 comments on commit f561705

Please sign in to comment.