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

Escape job ID in API requests (#2411) 

Jobs can be created with user-provided IDs containing any character
except spaces. The jobId needs to be escaped when used in a request
path, otherwise jobs created with names such as "why?" can't be managed
after they are created.
This commit is contained in:
Ben Barnard 2019-11-07 14:35:39 +01:00 committed by Tim Gross
parent 3216c62cdb
commit b87ecd5f8c
3 changed files with 24 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
api/api.go
View File

@ -592,10 +592,11 @@ func (c *Client) newRequest(method, path string) (*request, error) {
config: &c.config, config: &c.config,
method: method, method: method,
url: &url.URL{ url: &url.URL{
Scheme: base.Scheme, Scheme: base.Scheme,
User: base.User, User: base.User,
Host: base.Host, Host: base.Host,
Path: u.Path, Path: u.Path,
RawPath: u.RawPath,
}, },
params: make(map[string][]string), params: make(map[string][]string),
} }

30
api/jobs.go
View File

@ -146,7 +146,7 @@ func (j *Jobs) PrefixList(prefix string) ([]*JobListStub, *QueryMeta, error) {
// job given its unique ID. // job given its unique ID.
func (j *Jobs) Info(jobID string, q *QueryOptions) (*Job, *QueryMeta, error) { func (j *Jobs) Info(jobID string, q *QueryOptions) (*Job, *QueryMeta, error) {
var resp Job var resp Job
qm, err := j.client.query("/v1/job/"+jobID, &resp, q) qm, err := j.client.query("/v1/job/"+url.PathEscape(jobID), &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -157,7 +157,7 @@ func (j *Jobs) Info(jobID string, q *QueryOptions) (*Job, *QueryMeta, error) {
// unique ID. // unique ID.
func (j *Jobs) Versions(jobID string, diffs bool, q *QueryOptions) ([]*Job, []*JobDiff, *QueryMeta, error) { func (j *Jobs) Versions(jobID string, diffs bool, q *QueryOptions) ([]*Job, []*JobDiff, *QueryMeta, error) {
var resp JobVersionsResponse var resp JobVersionsResponse
qm, err := j.client.query(fmt.Sprintf("/v1/job/%s/versions?diffs=%v", jobID, diffs), &resp, q) qm, err := j.client.query(fmt.Sprintf("/v1/job/%s/versions?diffs=%v", url.PathEscape(jobID), diffs), &resp, q)
if err != nil { if err != nil {
return nil, nil, nil, err return nil, nil, nil, err
} }
@ -167,7 +167,7 @@ func (j *Jobs) Versions(jobID string, diffs bool, q *QueryOptions) ([]*Job, []*J
// Allocations is used to return the allocs for a given job ID. // Allocations is used to return the allocs for a given job ID.
func (j *Jobs) Allocations(jobID string, allAllocs bool, q *QueryOptions) ([]*AllocationListStub, *QueryMeta, error) { func (j *Jobs) Allocations(jobID string, allAllocs bool, q *QueryOptions) ([]*AllocationListStub, *QueryMeta, error) {
var resp []*AllocationListStub var resp []*AllocationListStub
u, err := url.Parse("/v1/job/" + jobID + "/allocations") u, err := url.Parse("/v1/job/" + url.PathEscape(jobID) + "/allocations")
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -188,7 +188,7 @@ func (j *Jobs) Allocations(jobID string, allAllocs bool, q *QueryOptions) ([]*Al
// ID. // ID.
func (j *Jobs) Deployments(jobID string, all bool, q *QueryOptions) ([]*Deployment, *QueryMeta, error) { func (j *Jobs) Deployments(jobID string, all bool, q *QueryOptions) ([]*Deployment, *QueryMeta, error) {
var resp []*Deployment var resp []*Deployment
u, err := url.Parse("/v1/job/" + jobID + "/deployments") u, err := url.Parse("/v1/job/" + url.PathEscape(jobID) + "/deployments")
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -208,7 +208,7 @@ func (j *Jobs) Deployments(jobID string, all bool, q *QueryOptions) ([]*Deployme
// the given job ID. // the given job ID.
func (j *Jobs) LatestDeployment(jobID string, q *QueryOptions) (*Deployment, *QueryMeta, error) { func (j *Jobs) LatestDeployment(jobID string, q *QueryOptions) (*Deployment, *QueryMeta, error) {
var resp *Deployment var resp *Deployment
qm, err := j.client.query("/v1/job/"+jobID+"/deployment", &resp, q) qm, err := j.client.query("/v1/job/"+url.PathEscape(jobID)+"/deployment", &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -219,7 +219,7 @@ func (j *Jobs) LatestDeployment(jobID string, q *QueryOptions) (*Deployment, *Qu
// ID. // ID.
func (j *Jobs) Evaluations(jobID string, q *QueryOptions) ([]*Evaluation, *QueryMeta, error) { func (j *Jobs) Evaluations(jobID string, q *QueryOptions) ([]*Evaluation, *QueryMeta, error) {
var resp []*Evaluation var resp []*Evaluation
qm, err := j.client.query("/v1/job/"+jobID+"/evaluations", &resp, q) qm, err := j.client.query("/v1/job/"+url.PathEscape(jobID)+"/evaluations", &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -232,7 +232,7 @@ func (j *Jobs) Evaluations(jobID string, q *QueryOptions) ([]*Evaluation, *Query
// eventually GC'ed from the system. Most callers should not specify purge. // eventually GC'ed from the system. Most callers should not specify purge.
func (j *Jobs) Deregister(jobID string, purge bool, q *WriteOptions) (string, *WriteMeta, error) { func (j *Jobs) Deregister(jobID string, purge bool, q *WriteOptions) (string, *WriteMeta, error) {
var resp JobDeregisterResponse var resp JobDeregisterResponse
wm, err := j.client.delete(fmt.Sprintf("/v1/job/%v?purge=%t", jobID, purge), &resp, q) wm, err := j.client.delete(fmt.Sprintf("/v1/job/%v?purge=%t", url.PathEscape(jobID), purge), &resp, q)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -242,7 +242,7 @@ func (j *Jobs) Deregister(jobID string, purge bool, q *WriteOptions) (string, *W
// ForceEvaluate is used to force-evaluate an existing job. // ForceEvaluate is used to force-evaluate an existing job.
func (j *Jobs) ForceEvaluate(jobID string, q *WriteOptions) (string, *WriteMeta, error) { func (j *Jobs) ForceEvaluate(jobID string, q *WriteOptions) (string, *WriteMeta, error) {
var resp JobRegisterResponse var resp JobRegisterResponse
wm, err := j.client.write("/v1/job/"+jobID+"/evaluate", nil, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/evaluate", nil, &resp, q)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -258,7 +258,7 @@ func (j *Jobs) EvaluateWithOpts(jobID string, opts EvalOptions, q *WriteOptions)
} }
var resp JobRegisterResponse var resp JobRegisterResponse
wm, err := j.client.write("/v1/job/"+jobID+"/evaluate", req, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/evaluate", req, &resp, q)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -268,7 +268,7 @@ func (j *Jobs) EvaluateWithOpts(jobID string, opts EvalOptions, q *WriteOptions)
// PeriodicForce spawns a new instance of the periodic job and returns the eval ID // PeriodicForce spawns a new instance of the periodic job and returns the eval ID
func (j *Jobs) PeriodicForce(jobID string, q *WriteOptions) (string, *WriteMeta, error) { func (j *Jobs) PeriodicForce(jobID string, q *WriteOptions) (string, *WriteMeta, error) {
var resp periodicForceResponse var resp periodicForceResponse
wm, err := j.client.write("/v1/job/"+jobID+"/periodic/force", nil, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/periodic/force", nil, &resp, q)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -301,7 +301,7 @@ func (j *Jobs) PlanOpts(job *Job, opts *PlanOptions, q *WriteOptions) (*JobPlanR
} }
var resp JobPlanResponse var resp JobPlanResponse
wm, err := j.client.write("/v1/job/"+*job.ID+"/plan", req, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(*job.ID)+"/plan", req, &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -310,7 +310,7 @@ func (j *Jobs) PlanOpts(job *Job, opts *PlanOptions, q *WriteOptions) (*JobPlanR
func (j *Jobs) Summary(jobID string, q *QueryOptions) (*JobSummary, *QueryMeta, error) { func (j *Jobs) Summary(jobID string, q *QueryOptions) (*JobSummary, *QueryMeta, error) {
var resp JobSummary var resp JobSummary
qm, err := j.client.query("/v1/job/"+jobID+"/summary", &resp, q) qm, err := j.client.query("/v1/job/"+url.PathEscape(jobID)+"/summary", &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -325,7 +325,7 @@ func (j *Jobs) Dispatch(jobID string, meta map[string]string,
Meta: meta, Meta: meta,
Payload: payload, Payload: payload,
} }
wm, err := j.client.write("/v1/job/"+jobID+"/dispatch", req, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/dispatch", req, &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -345,7 +345,7 @@ func (j *Jobs) Revert(jobID string, version uint64, enforcePriorVersion *uint64,
EnforcePriorVersion: enforcePriorVersion, EnforcePriorVersion: enforcePriorVersion,
VaultToken: vaultToken, VaultToken: vaultToken,
} }
wm, err := j.client.write("/v1/job/"+jobID+"/revert", req, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/revert", req, &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -362,7 +362,7 @@ func (j *Jobs) Stable(jobID string, version uint64, stable bool,
JobVersion: version, JobVersion: version,
Stable: stable, Stable: stable,
} }
wm, err := j.client.write("/v1/job/"+jobID+"/stable", req, &resp, q) wm, err := j.client.write("/v1/job/"+url.PathEscape(jobID)+"/stable", req, &resp, q)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }

6
api/jobs_test.go
View File

@ -876,13 +876,15 @@ func TestJobs_Info(t *testing.T) {
// Trying to retrieve a job by ID before it exists // Trying to retrieve a job by ID before it exists
// returns an error // returns an error
_, _, err := jobs.Info("job1", nil) id := "job-id/with\\troublesome:characters\n?&字\000"
_, _, err := jobs.Info(id, nil)
if err == nil || !strings.Contains(err.Error(), "not found") { if err == nil || !strings.Contains(err.Error(), "not found") {
t.Fatalf("expected not found error, got: %#v", err) t.Fatalf("expected not found error, got: %#v", err)
} }
// Register the job // Register the job
job := testJob() job := testJob()
job.ID = &id
_, wm, err := jobs.Register(job, nil) _, wm, err := jobs.Register(job, nil)
if err != nil { if err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
@ -890,7 +892,7 @@ func TestJobs_Info(t *testing.T) {
assertWriteMeta(t, wm) assertWriteMeta(t, wm)
// Query the job again and ensure it exists // Query the job again and ensure it exists
result, qm, err := jobs.Info("job1", nil) result, qm, err := jobs.Info(id, nil)
if err != nil { if err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }