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

Disable http2 for indexServer #191

Merged
merged 10 commits into from
Nov 28, 2023
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: 2 additions & 2 deletions build_registry.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# This can be useful if developing components within this repository (such as the index server or build tools)
# and want to test all of the components together
shopt -s expand_aliases
set -eux
set -ex

# Set base registry support directory
BASE_DIR=$(dirname $0)
Expand All @@ -28,7 +28,7 @@ BASE_DIR=$(dirname $0)
. ${BASE_DIR}/setenv.sh

# Build the index server base image
. ${BASE_DIR}/index/server/build.sh
ENABLE_HTTP2=${ENABLE_HTTP2} . ${BASE_DIR}/index/server/build.sh

# Build the test devfile registry image
docker build -t devfile-index:latest -f ${BASE_DIR}/.ci/Dockerfile ${BASE_DIR}
4 changes: 4 additions & 0 deletions index/server/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ RUN set -x ; \
# Modify the permissions on the necessary files to allow the container to properly run as a non-root UID
RUN mkdir -p /www/data && chmod -R g+rwx /www/data

# disable http/2 on the index server by default
ARG ENABLE_HTTP2=false
ENV ENABLE_HTTP2=${ENABLE_HTTP2}

# Set env vars for the locations of the devfile stacks and index.json
ENV DEVFILE_STACKS /registry/stacks
ENV DEVFILE_SAMPLES /registry/samples
Expand Down
55 changes: 31 additions & 24 deletions index/server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,26 @@ Edit the OpenAPI spec `openapi.yaml`, under `paths` you can define your endpoint

```yaml
paths:
/foo:
get:
summary: <short summary of what your endpoint does>
description: <a long description of what your endpoint does>
# 'serveFoo' points to handler function 'ServeFoo'
operationId: serveFoo
parameters: # the OpenAPI specifications of the endpoint parameters
# spec for passing a bar query parameter /foo?bar=<string>
- name: bar
in: query
description: <description for parameter>
required: false
schema:
type: string
responses: # the OpenAPI specifications for the endpoint responses
default:
description: <description of the response>
content:
# Content type(s)
text/html: {}
/foo:
get:
summary: <short summary of what your endpoint does>
description: <a long description of what your endpoint does>
# 'serveFoo' points to handler function 'ServeFoo'
operationId: serveFoo
parameters: # the OpenAPI specifications of the endpoint parameters
# spec for passing a bar query parameter /foo?bar=<string>
- name: bar
in: query
description: <description for parameter>
required: false
schema:
type: string
responses: # the OpenAPI specifications for the endpoint responses
default:
description: <description of the response>
content:
# Content type(s)
text/html: {}
```

See [swagger.io/docs](https://swagger.io/docs/specification/paths-and-operations) for more information.
Expand Down Expand Up @@ -72,20 +72,27 @@ bash push.sh quay.io/someuser/devfile-index-base

### Source Generation

Index server build uses the CLI tool `oapi-codegen` to generate the schema types `pkg/server/types.gen.go` and endpoint definition `pkg/server/endpoint.gen.go` sources. When changing the OpenAPI specification, such as [defining endpoints](#defining-endpoints), it is required to regenerate these changes into the source.
Index server build uses the CLI tool `oapi-codegen` to generate the schema types `pkg/server/types.gen.go` and endpoint definition `pkg/server/endpoint.gen.go` sources. When changing the OpenAPI specification, such as [defining endpoints](#defining-endpoints), it is required to regenerate these changes into the source.

The source generation can be done by manually building the index server with:
The source generation can be done by manually building the index server with:

```bash
bash build.sh
```

or to just generate the source files by running:

```bash
bash codegen.sh
```

**Important**: When committing to this repository, it is *required* to include the up to date source generation in your pull requests. Not including up to date source generation will lead to the PR check to fail.
**Important**: When committing to this repository, it is _required_ to include the up to date source generation in your pull requests. Not including up to date source generation will lead to the PR check to fail.

### Enabling HTTP/2 on the Index Server

By default, http/2 on the index server is disabled due to [CVE-2023-44487](https://github.com/advisories/GHSA-qppj-fm5r-hxr3).

If you want to enable http/2, build with `ENABLE_HTTP2=true bash build.sh`.

## Testing

Expand All @@ -104,4 +111,4 @@ go test ./...
**Environment Variables**

- `DEVFILE_REGISTRY`: Optional environment variable for specifying testing registry path
- default: `../../tests/registry`
- default: `../../tests/registry`
2 changes: 1 addition & 1 deletion index/server/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ buildfolder="$(realpath $(dirname ${BASH_SOURCE[0]}))"
bash ${buildfolder}/codegen.sh

# Build the index server
docker build -t devfile-index-base:latest $buildfolder
docker build -t devfile-index-base:latest --build-arg ENABLE_HTTP2=${ENABLE_HTTP2} $buildfolder
19 changes: 19 additions & 0 deletions index/server/pkg/server/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
package server

import (
"crypto/tls"
"encoding/json"
"io/ioutil"
"log"
"net/http"
"os"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -67,13 +69,30 @@
handler := http.NewServeMux()
handler.Handle("/metrics", promhttp.Handler())
prometheus.MustRegister(getIndexLatency)

// Retrieve the option to enable HTTP2
enableHTTP2 := os.Getenv("ENABLE_HTTP2")
if enableHTTP2 == "" {
enableHTTP2 = "false"
}

Check warning on line 77 in index/server/pkg/server/index.go

View check run for this annotation

Codecov / codecov/patch

index/server/pkg/server/index.go#L72-L77

Added lines #L72 - L77 were not covered by tests

indexServer := &http.Server{
Addr: ":7071",
Handler: handler,
ReadTimeout: 10 * time.Second,
WriteTimeout: 10 * time.Second,
}

// Disable HTTP2 by default
if enableHTTP2 == "false" {
indexServer.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
}

Check warning on line 89 in index/server/pkg/server/index.go

View check run for this annotation

Codecov / codecov/patch

index/server/pkg/server/index.go#L86-L89

Added lines #L86 - L89 were not covered by tests

// Disable HTTP2 by default
if enableHTTP2 == "false" {
indexServer.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
}

Check warning on line 94 in index/server/pkg/server/index.go

View check run for this annotation

Codecov / codecov/patch

index/server/pkg/server/index.go#L92-L94

Added lines #L92 - L94 were not covered by tests

go indexServer.ListenAndServe()

// Wait until registry is up and running
Expand Down
Loading