Skip to content

server: Drop HTML index page #8478

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.

Sign up for GitHub

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

Open
johanfylling wants to merge 4 commits into
base: main
Choose a base branch
from
+0 −33
Open

server: Drop HTML index page #8478

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6132e8
server: Drop HTML index page
johanfylling Apr 2, 2026
d21257b
always return a 200 OK
johanfylling Apr 2, 2026
797213e
Adding back 204 to `v1/data` PATCH
johanfylling Apr 2, 2026
a14f3df
Adding back index page with version fields
johanfylling Apr 9, 2026
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
71 changes: 1 addition & 70 deletions v1/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"encoding/json"
"errors"
"fmt"
"html/template"
"io"
"net"
"net/http"
Expand Down Expand Up @@ -1008,17 +1007,7 @@ func (s *Server) execQuery(ctx context.Context, br bundleRevisions, txn storage.
}

func (*Server) indexGet(w http.ResponseWriter, _ *http.Request) {
_ = indexHTML.Execute(w, struct {
Version string
BuildCommit string
BuildTimestamp string
BuildHostname string
}{
Version: version.Version,
BuildCommit: version.Vcs,
BuildTimestamp: version.Timestamp,
BuildHostname: version.Hostname,
})
w.WriteHeader(http.StatusOK)
Copy link
Copy Markdown
Contributor

@srenatus srenatus Apr 2, 2026

Choose a reason for hiding this comment

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

[nit] I think that's what stdlib will do if we just do nothing here?

Copy link
Copy Markdown
Contributor Author

@johanfylling johanfylling Apr 2, 2026

Choose a reason for hiding this comment

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

Then we get a 405 Method Not Allowed. So if that's the default behavior we're doing something to disable it.

Copy link
Copy Markdown
Contributor

@srenatus srenatus Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

❗ OK TIL. 🤔

}

type bundleRevisions struct {
Expand Down Expand Up @@ -1712,7 +1701,6 @@ func (s *Server) v1DataPatch(w http.ResponseWriter, r *http.Request) {
return
}

w.WriteHeader(http.StatusNoContent)
Copy link
Copy Markdown
Contributor Author

@johanfylling johanfylling Apr 2, 2026

Choose a reason for hiding this comment

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

Oops

}

func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -3012,63 +3000,6 @@ func readInputCompilePostV1(reqBytes []byte, queryParserOptions ast.ParserOption
}, nil
}

var indexHTML, _ = template.New("index").Parse(`
<html>
<head>
<script type="text/javascript">
function query() {
params = {
'query': document.getElementById("query").value,
}
if (document.getElementById("input").value !== "") {
try {
params["input"] = JSON.parse(document.getElementById("input").value);
} catch (e) {
document.getElementById("result").innerHTML = e;
return;
}
}
body = JSON.stringify(params);
opts = {
'method': 'POST',
'body': body,
}
fetch(new Request('v1/query', opts))
.then(resp => resp.json())
.then(json => {
str = JSON.stringify(json, null, 2);
document.getElementById("result").innerHTML = str;
});
}
</script>
</head>
</body>
<pre>
________ ________ ________
|\ __ \ |\ __ \ |\ __ \
\ \ \|\ \ \ \ \|\ \ \ \ \|\ \
\ \ \\\ \ \ \ ____\ \ \ __ \
\ \ \\\ \ \ \ \___| \ \ \ \ \
\ \_______\ \ \__\ \ \__\ \__\
\|_______| \|__| \|__|\|__|
</pre>
Open Policy Agent - An open source project to policy-enable your service.<br>
Copy link
Copy Markdown
Contributor Author

@johanfylling johanfylling Apr 2, 2026

Choose a reason for hiding this comment

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

I suppose we could also keep this, and just drop the query form (and maybe the info fields) below ..

Copy link
Copy Markdown
Contributor

@srenatus srenatus Apr 2, 2026

Choose a reason for hiding this comment

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

What do we gain? We're only telling the world what this server is. I don't think there's any benefit here.

Copy link
Copy Markdown
Contributor Author

@johanfylling johanfylling Apr 2, 2026

Choose a reason for hiding this comment

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

True. Looks like we have some failing tests because of dropping the endpoint, so maybe someone else is using it for health/liveliness checks despite there being dedicated endpoints for that 🤷.

Copy link
Copy Markdown
Contributor

@srenatus srenatus Apr 2, 2026

Choose a reason for hiding this comment

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

The health endpoint isn't exactly new, so I think the empty-body 200 is a good compromise.

<br>
Version: {{ .Version }}<br>
Build Commit: {{ .BuildCommit }}<br>
Build Timestamp: {{ .BuildTimestamp }}<br>
Build Hostname: {{ .BuildHostname }}<br>
<br>
Copy link
Copy Markdown
Contributor

@charlieegan3 charlieegan3 Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that removing the form makes sense, but I might vote to keep the Commit and Version as I am unsure how you might do this without running rego otherwise and people might depend on that if not the form?

Release #1, #2, ..#n

  1. We can remove the form. Add deprecation text to page.
  2. Still 200, but remove the version etc.
  3. Remove the page. but still show a 200OK and {"status": "OK"}

Copy link
Copy Markdown
Contributor Author

@johanfylling johanfylling Apr 7, 2026

Choose a reason for hiding this comment

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

I'm ok with only dropping the form 👍.
Looks like maybe only the version is part of the /status endpoint's returned labels; maybe we should add these other fields too(?).

Copy link
Copy Markdown
Contributor

@charlieegan3 charlieegan3 Apr 9, 2026

Choose a reason for hiding this comment

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

Yeah I think that makes sense as a continuity gesture at least. It doesn't look like it's possible to change the version label or the id one either, would these become the same? it might be that someone is using hostname in their own labels.

Query:<br>
<textarea rows="10" cols="50" id="query"></textarea><br>
<br>Input Data (JSON):<br>
<textarea rows="10" cols="50" id="input"></textarea><br>
<br><button onclick="query()">Submit</button>
<pre><div id="result"></div></pre>
</body>
</html>
`)

type decisionLogger struct {
revisions map[string]string
revision string // Deprecated: Use `revisions` instead.
Expand Down
Loading