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

Deck: Optionally allow to make the rerun button create a new Job #12827

Merged
merged 2 commits into from
Jun 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions prow/cluster/deck_rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ rules:
verbs:
- get
- list
# Required when deck runs with `--rerun-creates-job=true`
# **Warning:** Only use this for non-public deck instances, this allows
# anyone with access to your Deck instance to create new Prowjobs
# - create
---
kind: Role
apiVersion: rbac.authorization.k8s.io/v1beta1
Expand Down
4 changes: 4 additions & 0 deletions prow/cluster/starter.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,10 @@ rules:
verbs:
- get
- list
# Required when deck runs with `--rerun-creates-job=true`
# **Warning:** Only use this for non-public deck instances, this allows
# anyone with access to your Deck instance to create new Prowjobs
# - create
---
kind: Role
apiVersion: rbac.authorization.k8s.io/v1beta1
Expand Down
30 changes: 25 additions & 5 deletions prow/cmd/deck/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ type options struct {
spyglass bool
spyglassFilesLocation string
gcsCredentialsFile string
rerunCreatesJob bool
}

func (o *options) Validate() error {
Expand Down Expand Up @@ -136,6 +137,7 @@ func gatherOptions(fs *flag.FlagSet, args ...string) options {
fs.StringVar(&o.staticFilesLocation, "static-files-location", "/static", "Path to the static files")
fs.StringVar(&o.templateFilesLocation, "template-files-location", "/template", "Path to the template files")
fs.StringVar(&o.gcsCredentialsFile, "gcs-credentials-file", "", "Path to the GCS credentials file")
fs.BoolVar(&o.rerunCreatesJob, "rerun-creates-job", false, "Change the re-run option in Deck to actually create the job. **WARNING:** Only use this with non-public deck instances, otherwise strangers can DOS your Prow instance")
o.kubernetes.AddFlags(fs)
fs.Parse(args)
o.configPath = config.ConfigPath(o.configPath)
Expand Down Expand Up @@ -267,7 +269,7 @@ func main() {
mux.Handle("/tide-history", gziphandler.GzipHandler(handleSimpleTemplate(o, cfg, "tide-history.html", nil)))
mux.Handle("/plugins", gziphandler.GzipHandler(handleSimpleTemplate(o, cfg, "plugins.html", nil)))

indexHandler := handleSimpleTemplate(o, cfg, "index.html", struct{ SpyglassEnabled bool }{o.spyglass})
indexHandler := handleSimpleTemplate(o, cfg, "index.html", struct{ SpyglassEnabled, ReRunCreatesJob bool }{o.spyglass, o.rerunCreatesJob})

runLocal := o.pregeneratedData != ""

Expand Down Expand Up @@ -392,7 +394,7 @@ func prodOnlyMain(cfg config.Getter, o options, mux *http.ServeMux) *http.ServeM
mux.Handle("/prowjobs.js", gziphandler.GzipHandler(handleProwJobs(ja)))
mux.Handle("/badge.svg", gziphandler.GzipHandler(handleBadge(ja)))
mux.Handle("/log", gziphandler.GzipHandler(handleLog(ja)))
mux.Handle("/rerun", gziphandler.GzipHandler(handleRerun(prowJobClient)))
mux.Handle("/rerun", gziphandler.GzipHandler(handleRerun(prowJobClient, o.rerunCreatesJob)))
mux.Handle("/prowjob", gziphandler.GzipHandler(handleProwJob(prowJobClient)))

if o.spyglass {
Expand Down Expand Up @@ -1147,7 +1149,7 @@ func handleProwJob(prowJobClient prowv1.ProwJobInterface) http.HandlerFunc {
}
}

func handleRerun(prowJobClient prowv1.ProwJobInterface) http.HandlerFunc {
func handleRerun(prowJobClient prowv1.ProwJobInterface, createProwJob bool) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
name := r.URL.Query().Get("prowjob")
if name == "" {
Expand All @@ -1163,8 +1165,26 @@ func handleRerun(prowJobClient prowv1.ProwJobInterface) http.HandlerFunc {
}
return
}
pjutil := pjutil.NewProwJob(pj.Spec, pj.ObjectMeta.Labels)
b, err := yaml.Marshal(&pjutil)
newPJ := pjutil.NewProwJob(pj.Spec, pj.ObjectMeta.Labels)
// Be very careful about this on publicly accessible Prow instances. Even after we have authentication
// for the handler, we need CSRF protection.
// On Prow instances that require auth even for viewing Deck this is okayish, because the Prowjob UUID
// is hard to guess
// Ref: https://github.com/kubernetes/test-infra/pull/12827#issuecomment-502850414
if createProwJob {
if r.Method != http.MethodPost {
http.Error(w, "request must be of type POST", http.StatusMethodNotAllowed)
return
}
if _, err := prowJobClient.Create(&newPJ); err != nil {
logrus.WithError(err).Error("Error creating job")
http.Error(w, fmt.Sprintf("Error creating job: %v", err), http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusNoContent)
return
}
b, err := yaml.Marshal(&newPJ)
if err != nil {
http.Error(w, fmt.Sprintf("Error marshaling: %v", err), http.StatusInternalServerError)
logrus.WithError(err).Error("Error marshaling jobs.")
Expand Down
117 changes: 75 additions & 42 deletions prow/cmd/deck/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,51 +256,84 @@ func TestProwJob(t *testing.T) {
// TestRerun just checks that the result can be unmarshaled properly, has an
// updated status, and has equal spec.
func TestRerun(t *testing.T) {
fakeProwJobClient := fake.NewSimpleClientset(&prowapi.ProwJob{
ObjectMeta: metav1.ObjectMeta{
Name: "wowsuch",
Namespace: "prowjobs",
},
Spec: prowapi.ProwJobSpec{
Job: "whoa",
Type: prowapi.PresubmitJob,
Refs: &prowapi.Refs{
Org: "org",
Repo: "repo",
Pulls: []prowapi.Pull{
{Number: 1},
},
},
testCases := []struct {
name string
shouldCreateProwjob bool
}{
{
name: "Handler returns ProwJob",
shouldCreateProwjob: false,
},
Status: prowapi.ProwJobStatus{
State: prowapi.PendingState,
{
name: "Handler creates ProwJob",
shouldCreateProwjob: true,
},
})
handler := handleRerun(fakeProwJobClient.ProwV1().ProwJobs("prowjobs"))
req, err := http.NewRequest(http.MethodGet, "/rerun?prowjob=wowsuch", nil)
if err != nil {
t.Fatalf("Error making request: %v", err)
}
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("Bad error code: %d", rr.Code)
}
resp := rr.Result()
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("Error reading response body: %v", err)
}
var res prowapi.ProwJob
if err := yaml.Unmarshal(body, &res); err != nil {
t.Fatalf("Error unmarshaling: %v", err)
}
if res.Spec.Job != "whoa" {
t.Errorf("Wrong job, expected \"whoa\", got \"%s\"", res.Spec.Job)
}
if res.Status.State != prowapi.TriggeredState {
t.Errorf("Wrong state, expected \"%v\", got \"%v\"", prowapi.TriggeredState, res.Status.State)

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fakeProwJobClient := fake.NewSimpleClientset(&prowapi.ProwJob{
ObjectMeta: metav1.ObjectMeta{
Name: "wowsuch",
Namespace: "prowjobs",
},
Spec: prowapi.ProwJobSpec{
Job: "whoa",
Type: prowapi.PresubmitJob,
Refs: &prowapi.Refs{
Org: "org",
Repo: "repo",
Pulls: []prowapi.Pull{
{Number: 1},
},
},
},
Status: prowapi.ProwJobStatus{
State: prowapi.PendingState,
},
})
handler := handleRerun(fakeProwJobClient.ProwV1().ProwJobs("prowjobs"), tc.shouldCreateProwjob)
req, err := http.NewRequest(http.MethodPost, "/rerun?prowjob=wowsuch", nil)
if err != nil {
t.Fatalf("Error making request: %v", err)
}
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)

if tc.shouldCreateProwjob {
if rr.Code != http.StatusNoContent {
t.Fatalf("Unexpected http status code: %d, expected 204", rr.Code)
}
pjs, err := fakeProwJobClient.ProwV1().ProwJobs("prowjobs").List(metav1.ListOptions{})
if err != nil {
t.Fatalf("failed to list prowjobs: %v", err)
}
if numPJs := len(pjs.Items); numPJs != 2 {
t.Errorf("expected to get two prowjobs, got %d", numPJs)
}

} else {
if rr.Code != http.StatusOK {
t.Fatalf("Bad error code: %d", rr.Code)
}
resp := rr.Result()
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("Error reading response body: %v", err)
}
var res prowapi.ProwJob
if err := yaml.Unmarshal(body, &res); err != nil {
t.Fatalf("Error unmarshaling: %v", err)
}
if res.Spec.Job != "whoa" {
t.Errorf("Wrong job, expected \"whoa\", got \"%s\"", res.Spec.Job)
}
if res.Status.State != prowapi.TriggeredState {
t.Errorf("Wrong state, expected \"%v\", got \"%v\"", prowapi.TriggeredState, res.Status.State)
}
}
})
}
}

Expand Down
19 changes: 16 additions & 3 deletions prow/cmd/deck/static/prow/prow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {JobHistogram, JobSample} from './histogram';

declare const allBuilds: Job[];
declare const spyglass: boolean;
declare const rerunCreatesJob: boolean;

// http://stackoverflow.com/a/5158301/3694
function getParameterByName(name: string): string | null {
Expand Down Expand Up @@ -653,16 +654,28 @@ function redraw(fz: FuzzySearch): void {
function createRerunCell(modal: HTMLElement, rerunElement: HTMLElement, prowjob: string): HTMLTableDataCellElement {
const url = `https://${window.location.hostname}/rerun?prowjob=${prowjob}`;
const c = document.createElement("td");
const i = icon.create("refresh", "Show instructions for rerunning this job");
i.onclick = () => {
let i;
if (rerunCreatesJob) {
i = icon.create("refresh", "Rerun this job");
i.onclick = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up - some people have said they'd still like to see the "kubectl create ..." popup, even if directly triggering the rerun is enabled. In my PR, I'm adding a "run" button within that popup that will actually rerun the job, so it's still displayed:
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mirandachrist If you just hadn't explained that, I wouldn't have derived from the view on the screenshot that the run button runs the job tbh (But that discussion is probably better put on your PR)

const form = document.createElement('form');
form.method = 'POST';
form.action = `${url}`;
c.appendChild(form);
form.submit();
};
} else {
i = icon.create("refresh", "Show instructions for rerunning this job");
i.onclick = () => {
modal.style.display = "block";
rerunElement.innerHTML = `kubectl create -f "<a href="${url}">${url}</a>"`;
const copyButton = document.createElement('a');
copyButton.className = "mdl-button mdl-js-button mdl-button--icon";
copyButton.onclick = () => copyToClipboardWithToast(`kubectl create -f "${url}"`);
copyButton.innerHTML = "<i class='material-icons state triggered' style='color: gray'>file_copy</i>";
rerunElement.appendChild(copyButton);
};
};
}
c.appendChild(i);
c.classList.add("icon-cell");
return c;
Expand Down
1 change: 1 addition & 0 deletions prow/cmd/deck/template/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<script type="text/javascript" src="data.js?var=allBuilds"></script>
<script type="text/javascript">
var spyglass = {{.SpyglassEnabled}};
var rerunCreatesJob = {{.ReRunCreatesJob}};
</script>
{{end}}

Expand Down