-
Notifications
You must be signed in to change notification settings - Fork 5
feat(web): initial work to setup root handler in the web component #2192
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
Changes from 11 commits
b891d92
5fa7eb6
edce3c0
cd42d23
e5314fb
e3baabd
6c80056
5c7fa49
bd6de36
bc5bf63
e44d3fd
5553e91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ import ( | |
"errors" | ||
"fmt" | ||
"net/http" | ||
"net/http/httptest" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
|
@@ -224,94 +223,3 @@ func TestAPIError_ErrorOrNil(t *testing.T) { | |
}) | ||
} | ||
} | ||
|
||
func TestAPIError_JSON(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some mismatch between PRs that have been merged, this fixes some go vet issues: |
||
tests := []struct { | ||
name string | ||
apiErr *APIError | ||
wantCode int | ||
wantJSON map[string]any | ||
}{ | ||
{ | ||
name: "simple error", | ||
apiErr: &APIError{ | ||
StatusCode: http.StatusInternalServerError, | ||
Message: "invalid request", | ||
}, | ||
wantCode: http.StatusInternalServerError, | ||
wantJSON: map[string]any{ | ||
"status_code": float64(http.StatusInternalServerError), | ||
"message": "invalid request", | ||
}, | ||
}, | ||
{ | ||
name: "field error", | ||
apiErr: &APIError{ | ||
StatusCode: http.StatusBadRequest, | ||
Message: "validation error", | ||
Field: "username", | ||
}, | ||
wantCode: http.StatusBadRequest, | ||
wantJSON: map[string]any{ | ||
"status_code": float64(http.StatusBadRequest), | ||
"message": "validation error", | ||
"field": "username", | ||
}, | ||
}, | ||
{ | ||
name: "error with nested errors", | ||
apiErr: &APIError{ | ||
StatusCode: http.StatusBadRequest, | ||
Message: "multiple validation errors", | ||
Errors: []*APIError{ | ||
{ | ||
Message: "field1 is required", | ||
Field: "field1", | ||
}, | ||
{ | ||
Message: "field2 must be a number", | ||
Field: "field2", | ||
}, | ||
}, | ||
}, | ||
wantCode: http.StatusBadRequest, | ||
wantJSON: map[string]any{ | ||
"status_code": float64(http.StatusBadRequest), | ||
"message": "multiple validation errors", | ||
"errors": []any{ | ||
map[string]any{ | ||
"message": "field1 is required", | ||
"field": "field1", | ||
}, | ||
map[string]any{ | ||
"message": "field2 must be a number", | ||
"field": "field2", | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
// Create a mock HTTP response recorder | ||
rec := httptest.NewRecorder() | ||
|
||
// Call the JSON method | ||
tt.apiErr.JSON(rec) | ||
|
||
// Check status code | ||
assert.Equal(t, tt.wantCode, rec.Code, "Status code should match") | ||
|
||
// Check content type header | ||
contentType := rec.Header().Get("Content-Type") | ||
assert.Equal(t, "application/json", contentType, "Content-Type header should be application/json") | ||
|
||
// Parse and check the JSON response | ||
var gotJSON map[string]any | ||
err := json.Unmarshal(rec.Body.Bytes(), &gotJSON) | ||
assert.NoError(t, err, "Should be able to parse the JSON response") | ||
assert.Equal(t, tt.wantJSON, gotJSON, "JSON response should match expected structure") | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import ( | |
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"io/fs" | ||
"log" | ||
"net" | ||
"net/http" | ||
|
@@ -97,6 +98,9 @@ type InstallCmdFlags struct { | |
tlsKeyBytes []byte | ||
} | ||
|
||
// webAssetsFS is the filesystem to be used by the web component. Defaults to nil allowing the web server to use the default assets embedded in the binary. Useful for testing. | ||
var webAssetsFS fs.FS = nil | ||
Comment on lines
+101
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not particularly happy about this, unsure if we should keep this somewhere else, open to ideas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than that README.md hack could you have a stub index.html? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main issue with that is when building the web assets, |
||
|
||
// InstallCmd returns a cobra command for installing the embedded cluster. | ||
func InstallCmd(ctx context.Context, name string) *cobra.Command { | ||
var flags InstallCmdFlags | ||
|
@@ -455,16 +459,21 @@ func runInstallAPI(ctx context.Context, listener net.Listener, cert tls.Certific | |
if err != nil { | ||
return fmt.Errorf("new api: %w", err) | ||
} | ||
app := release.GetApplication() | ||
if app == nil { | ||
return fmt.Errorf("application not found") | ||
} | ||
|
||
api.RegisterRoutes(router.PathPrefix("/api").Subrouter()) | ||
|
||
var webFs http.Handler | ||
if os.Getenv("EC_DEV_ENV") == "true" { | ||
webFs = http.FileServer(http.FS(os.DirFS("./web/dist"))) | ||
} else { | ||
webFs = http.FileServer(http.FS(web.Fs())) | ||
webServer, err := web.New(web.InitialState{ | ||
Title: app.Spec.Title, | ||
Icon: app.Spec.Icon, | ||
}, web.WithLogger(logger), web.WithAssetsFS(webAssetsFS)) | ||
if err != nil { | ||
return fmt.Errorf("new web server: %w", err) | ||
} | ||
router.PathPrefix("/").Methods("GET").Handler(webFs) | ||
|
||
api.RegisterRoutes(router.PathPrefix("/api").Subrouter()) | ||
webServer.RegisterRoutes(router.PathPrefix("/").Subrouter()) | ||
|
||
server := &http.Server{ | ||
// ErrorLog outputs TLS errors and warnings to the console, we want to make sure we use the same logrus logger for them | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
"os" | ||
"path/filepath" | ||
"testing" | ||
"testing/fstest" | ||
"time" | ||
|
||
"github.com/replicatedhq/embedded-cluster/api" | ||
|
@@ -560,6 +561,29 @@ func Test_runInstallAPI(t *testing.T) { | |
certPool := x509.NewCertPool() | ||
certPool.AddCert(cert.Leaf) | ||
|
||
// We need a release object to pass over to the Web component. | ||
dataMap := map[string][]byte{ | ||
"kots-app.yaml": []byte(` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should probably include an icon and a title There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can, albeit it's not really being used right now. Unless we also want to run some requests within this test against the web components? |
||
apiVersion: kots.io/v1beta1 | ||
kind: Application | ||
`), | ||
} | ||
err = release.SetReleaseDataForTests(dataMap) | ||
require.NoError(t, err) | ||
|
||
t.Cleanup(func() { | ||
release.SetReleaseDataForTests(nil) | ||
}) | ||
|
||
// Mock the web assets filesystem so that we don't need to embed the web assets. | ||
webAssetsFS = fstest.MapFS{ | ||
"index.html": &fstest.MapFile{ | ||
Data: []byte(""), | ||
Mode: 0644, | ||
}, | ||
} | ||
defer func() { webAssetsFS = nil }() | ||
|
||
go func() { | ||
err := runInstallAPI(ctx, listener, cert, logger, "password", nil) | ||
t.Logf("Install API exited with error: %v", err) | ||
|
@@ -581,7 +605,7 @@ func Test_runInstallAPI(t *testing.T) { | |
}, | ||
} | ||
resp, err := httpClient.Get(url) | ||
assert.NoError(t, err) | ||
require.NoError(t, err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Later it would panic leading to a red herring because there's no |
||
if resp != nil { | ||
defer resp.Body.Close() | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Keeping this file as a placeholder for //got:embed dist to work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer need the branding routes.