From 7a0ea5b244399ee9add0b0fb34f6706138f55841 Mon Sep 17 00:00:00 2001 From: sanne Date: Fri, 23 Jul 2021 10:10:12 +0200 Subject: [PATCH] worker: Remove identity filter Partially reverts "0ea31c39d577fc1bc59508d24d2287f125251ce1" --- cmd/osbuild-composer/composer.go | 26 +++++------- cmd/osbuild-composer/config.go | 1 - internal/cloudapi/server.go | 2 +- internal/mocks/rpmmd/fixtures.go | 2 +- internal/test/helpers.go | 13 +----- internal/worker/api/api.go | 1 - internal/worker/client.go | 6 +-- internal/worker/server.go | 72 +++++--------------------------- internal/worker/server_test.go | 56 +++++-------------------- 9 files changed, 36 insertions(+), 143 deletions(-) diff --git a/cmd/osbuild-composer/composer.go b/cmd/osbuild-composer/composer.go index 410332ce6..8eae9ba1e 100644 --- a/cmd/osbuild-composer/composer.go +++ b/cmd/osbuild-composer/composer.go @@ -84,7 +84,7 @@ func NewComposer(config *ComposerConfigFile, stateDir, cacheDir string, logger * } } - c.workers = worker.NewServer(c.logger, jobs, artifactsDir, c.config.Worker.IdentityFilter) + c.workers = worker.NewServer(c.logger, jobs, artifactsDir) return &c, nil } @@ -128,22 +128,18 @@ func (c *Composer) InitLocalWorker(l net.Listener) { } func (c *Composer) InitRemoteWorkers(cert, key string, l net.Listener) error { - if len(c.config.Worker.IdentityFilter) > 0 { - c.workerListener = l - } else { - tlsConfig, err := createTLSConfig(&connectionConfig{ - CACertFile: c.config.Worker.CA, - ServerKeyFile: key, - ServerCertFile: cert, - AllowedDomains: c.config.Worker.AllowedDomains, - }) - if err != nil { - return fmt.Errorf("Error creating TLS configuration for remote worker API: %v", err) - } - - c.workerListener = tls.NewListener(l, tlsConfig) + tlsConfig, err := createTLSConfig(&connectionConfig{ + CACertFile: c.config.Worker.CA, + ServerKeyFile: key, + ServerCertFile: cert, + AllowedDomains: c.config.Worker.AllowedDomains, + }) + if err != nil { + return fmt.Errorf("Error creating TLS configuration for remote worker API: %v", err) } + c.workerListener = tls.NewListener(l, tlsConfig) + return nil } diff --git a/cmd/osbuild-composer/config.go b/cmd/osbuild-composer/config.go index 739a38ef2..f885888a3 100644 --- a/cmd/osbuild-composer/config.go +++ b/cmd/osbuild-composer/config.go @@ -17,7 +17,6 @@ type ComposerConfigFile struct { Worker struct { AllowedDomains []string `toml:"allowed_domains"` CA string `toml:"ca"` - IdentityFilter []string `toml:"identity_filter"` PGHost string `toml:"pg_host" env:"PGHOST"` PGPort string `toml:"pg_port" env:"PGPORT"` PGDatabase string `toml:"pg_database" env:"PGDATABASE"` diff --git a/internal/cloudapi/server.go b/internal/cloudapi/server.go index 645aab5ba..36fe0aaef 100644 --- a/internal/cloudapi/server.go +++ b/internal/cloudapi/server.go @@ -101,7 +101,7 @@ func (server *Server) VerifyIdentityHeader(next echo.HandlerFunc) echo.HandlerFu return echo.NewHTTPError(http.StatusNotFound, "Auth header has incorrect format") } - var idHeader identityHeader + var idHeader IdentityHeader err = json.Unmarshal([]byte(strings.TrimSuffix(fmt.Sprintf("%s", b64Result), "\n")), &idHeader) if err != nil { return echo.NewHTTPError(http.StatusNotFound, "Auth header has incorrect format") diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go index f2bbea684..9f1fc2bc9 100644 --- a/internal/mocks/rpmmd/fixtures.go +++ b/internal/mocks/rpmmd/fixtures.go @@ -57,7 +57,7 @@ func createBaseWorkersFixture(tmpdir string) *worker.Server { if err != nil { panic(err) } - return worker.NewServer(nil, q, "", []string{}) + return worker.NewServer(nil, q, "") } func createBaseDepsolveFixture() []rpmmd.PackageSpec { diff --git a/internal/test/helpers.go b/internal/test/helpers.go index e4e9e948c..a194e33f1 100644 --- a/internal/test/helpers.go +++ b/internal/test/helpers.go @@ -50,14 +50,9 @@ func externalRequest(method, path, body string) *http.Response { return resp } -func internalRequest(api http.Handler, method, path, body string, header map[string]string) *http.Response { +func internalRequest(api http.Handler, method, path, body string) *http.Response { req := httptest.NewRequest(method, path, bytes.NewReader([]byte(body))) req.Header.Set("Content-Type", "application/json") - - for k, h := range header { - req.Header.Set(k, h) - } - resp := httptest.NewRecorder() api.ServeHTTP(resp, req) @@ -71,14 +66,10 @@ func SendHTTP(api http.Handler, external bool, method, path, body string) *http. } return externalRequest(method, path, body) } else { - return internalRequest(api, method, path, body, map[string]string{}) + return internalRequest(api, method, path, body) } } -func SendHTTPWithHeader(api http.Handler, method, path, body string, header map[string]string) *http.Response { - return internalRequest(api, method, path, body, header) -} - // this function serves to drop fields that shouldn't be tested from the unmarshalled json objects func dropFields(obj interface{}, fields ...string) { switch v := obj.(type) { diff --git a/internal/worker/api/api.go b/internal/worker/api/api.go index 72bf9a544..d379f8f2a 100644 --- a/internal/worker/api/api.go +++ b/internal/worker/api/api.go @@ -3,4 +3,3 @@ package api const BasePath = "/api/worker/v1" -const CloudBasePath = "/api/composer-worker/v1" diff --git a/internal/worker/client.go b/internal/worker/client.go index 3fb2fc970..f415e5f57 100644 --- a/internal/worker/client.go +++ b/internal/worker/client.go @@ -61,11 +61,7 @@ func NewClient(baseURL string, conf *tls.Config, offlineToken, oAuthURL *string) return nil, err } - bp := api.BasePath - if offlineToken != nil { - bp = api.CloudBasePath - } - server, err = server.Parse(bp + "/") + server, err = server.Parse(api.BasePath + "/") if err != nil { panic(err) } diff --git a/internal/worker/server.go b/internal/worker/server.go index 3a73c7a86..84ca25284 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -2,7 +2,6 @@ package worker import ( "context" - "encoding/base64" "encoding/json" "errors" "fmt" @@ -12,7 +11,6 @@ import ( "net/http" "os" "path" - "strings" "time" "github.com/google/uuid" @@ -24,10 +22,9 @@ import ( ) type Server struct { - jobs jobqueue.JobQueue - logger *log.Logger - artifactsDir string - identityFilter []string + jobs jobqueue.JobQueue + logger *log.Logger + artifactsDir string } type JobStatus struct { @@ -40,12 +37,11 @@ type JobStatus struct { var ErrInvalidToken = errors.New("token does not exist") var ErrJobNotRunning = errors.New("job isn't running") -func NewServer(logger *log.Logger, jobs jobqueue.JobQueue, artifactsDir string, identityFilter []string) *Server { +func NewServer(logger *log.Logger, jobs jobqueue.JobQueue, artifactsDir string) *Server { s := &Server{ - jobs: jobs, - logger: logger, - artifactsDir: artifactsDir, - identityFilter: identityFilter, + jobs: jobs, + logger: logger, + artifactsDir: artifactsDir, } go s.WatchHeartbeats() return s @@ -62,57 +58,14 @@ func (s *Server) Handler() http.Handler { e.DefaultHTTPErrorHandler(err, c) } - var mws []echo.MiddlewareFunc - if len(s.identityFilter) > 0 { - mws = append(mws, s.VerifyIdentityHeader) - } - handler := apiHandlers{ server: s, } - api.RegisterHandlers(e.Group(api.BasePath, mws...), &handler) - api.RegisterHandlers(e.Group(api.CloudBasePath, mws...), &handler) + api.RegisterHandlers(e.Group(api.BasePath), &handler) return e } -func (s *Server) VerifyIdentityHeader(nextHandler echo.HandlerFunc) echo.HandlerFunc { - return func(ctx echo.Context) error { - type identityHeader struct { - Identity struct { - AccountNumber string `json:"account_number"` - } `json:"identity"` - } - - request := ctx.Request() - - idHeaderB64 := request.Header["X-Rh-Identity"] - if len(idHeaderB64) != 1 { - return echo.NewHTTPError(http.StatusNotFound, "Auth header is not present") - } - - b64Result, err := base64.StdEncoding.DecodeString(idHeaderB64[0]) - if err != nil { - return echo.NewHTTPError(http.StatusNotFound, "Auth header has incorrect format") - } - - var idHeader identityHeader - err = json.Unmarshal([]byte(strings.TrimSuffix(fmt.Sprintf("%s", b64Result), "\n")), &idHeader) - if err != nil { - return echo.NewHTTPError(http.StatusNotFound, "Auth header has incorrect format") - } - - for _, i := range s.identityFilter { - if idHeader.Identity.AccountNumber == i { - ctx.Set("IdentityHeader", idHeader) - return nextHandler(ctx) - - } - } - return echo.NewHTTPError(http.StatusNotFound, "Account not allowed") - } -} - // This function should be started as a goroutine // Every 30 seconds it goes through all running jobs, removing any unresponsive ones. // It fails jobs which fail to check if they cancelled for more than 2 minutes. @@ -349,15 +302,10 @@ func (h *apiHandlers) RequestJob(ctx echo.Context) error { return err } - basePath := api.BasePath - if strings.HasPrefix(ctx.Path(), api.CloudBasePath) { - basePath = api.CloudBasePath - } - return ctx.JSON(http.StatusCreated, requestJobResponse{ Id: jobId, - Location: fmt.Sprintf("%s/jobs/%v", basePath, token), - ArtifactLocation: fmt.Sprintf("%s/jobs/%v/artifacts/", basePath, token), + Location: fmt.Sprintf("%s/jobs/%v", api.BasePath, token), + ArtifactLocation: fmt.Sprintf("%s/jobs/%v/artifacts/", api.BasePath, token), Type: jobType, Args: jobArgs, DynamicArgs: dynamicJobArgs, diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index 76d620e0c..94cff60c7 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/google/uuid" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/osbuild/osbuild-composer/internal/distro" @@ -22,12 +21,12 @@ import ( "github.com/osbuild/osbuild-composer/internal/worker" ) -func newTestServer(t *testing.T, tempdir string, identities []string) *worker.Server { +func newTestServer(t *testing.T, tempdir string) *worker.Server { q, err := fsjobqueue.New(tempdir) if err != nil { t.Fatalf("error creating fsjobqueue: %v", err) } - return worker.NewServer(nil, q, "", identities) + return worker.NewServer(nil, q, "") } // Ensure that the status request returns OK. @@ -36,7 +35,7 @@ func TestStatus(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tempdir) - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() test.TestRoute(t, handler, false, "GET", "/api/worker/v1/status", ``, http.StatusOK, `{"status":"OK"}`, "message") } @@ -67,7 +66,7 @@ func TestErrors(t *testing.T) { defer os.RemoveAll(tempdir) for _, c := range cases { - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() test.TestRoute(t, handler, false, c.Method, c.Path, c.Body, c.ExpectedStatus, "{}", "message") } @@ -91,7 +90,7 @@ func TestCreate(t *testing.T) { if err != nil { t.Fatalf("error creating osbuild manifest: %v", err) } - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() _, err = server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) @@ -120,7 +119,7 @@ func TestCancel(t *testing.T) { if err != nil { t.Fatalf("error creating osbuild manifest: %v", err) } - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) @@ -161,7 +160,7 @@ func TestUpdate(t *testing.T) { if err != nil { t.Fatalf("error creating osbuild manifest: %v", err) } - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) @@ -190,7 +189,7 @@ func TestArgs(t *testing.T) { tempdir, err := ioutil.TempDir("", "worker-tests-") require.NoError(t, err) defer os.RemoveAll(tempdir) - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) job := worker.OSBuildJob{ Manifest: manifest, @@ -230,7 +229,7 @@ func TestUpload(t *testing.T) { if err != nil { t.Fatalf("error creating osbuild manifest: %v", err) } - server := newTestServer(t, tempdir, []string{}) + server := newTestServer(t, tempdir) handler := server.Handler() jobID, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) @@ -246,41 +245,6 @@ func TestUpload(t *testing.T) { test.TestRoute(t, handler, false, "PUT", fmt.Sprintf("/api/worker/v1/jobs/%s/artifacts/foobar", token), `this is my artifact`, http.StatusOK, `?`) } -func TestIdentities(t *testing.T) { - tempdir, err := ioutil.TempDir("", "worker-tests-") - require.NoError(t, err) - defer os.RemoveAll(tempdir) - - // distroStruct := test_distro.New() - // arch, err := distroStruct.GetArch(test_distro.TestArchName) - // require.NoError(t, err) - // imageType, err := arch.GetImageType(test_distro.TestImageTypeName) - // require.NoError(t, err) - // manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, 0) - // require.NoError(t, err) - - server := newTestServer(t, tempdir, []string{"000000"}) - handler := server.Handler() - - // _, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) - // require.NoError(t, err) - - test.TestRoute(t, handler, false, "GET", "/api/worker/v1/status", ``, http.StatusNotFound, `{"message":"Auth header is not present"}`, "message") - - header := map[string]string{ - "x-rh-identity": "eyJlbnRpdGxlbWVudHMiOnsiaW5zaWdodHMiOnsiaXNfZW50aXRsZWQiOnRydWV9LCJzbWFydF9tYW5hZ2VtZW50Ijp7ImlzX2VudGl0bGVkIjp0cnVlfSwib3BlbnNoaWZ0Ijp7ImlzX2VudGl0bGVkIjp0cnVlfSwiaHlicmlkIjp7ImlzX2VudGl0bGVkIjp0cnVlfSwibWlncmF0aW9ucyI6eyJpc19lbnRpdGxlZCI6dHJ1ZX0sImFuc2libGUiOnsiaXNfZW50aXRsZWQiOnRydWV9fSwiaWRlbnRpdHkiOnsiYWNjb3VudF9udW1iZXIiOiIwMDAwMDMiLCJ0eXBlIjoiVXNlciIsInVzZXIiOnsidXNlcm5hbWUiOiJ1c2VyIiwiZW1haWwiOiJ1c2VyQHVzZXIudXNlciIsImZpcnN0X25hbWUiOiJ1c2VyIiwibGFzdF9uYW1lIjoidXNlciIsImlzX2FjdGl2ZSI6dHJ1ZSwiaXNfb3JnX2FkbWluIjp0cnVlLCJpc19pbnRlcm5hbCI6dHJ1ZSwibG9jYWxlIjoiZW4tVVMifSwiaW50ZXJuYWwiOnsib3JnX2lkIjoiMDAwMDAwIn19fQo=", - } - response := test.SendHTTPWithHeader(handler, "GET", "/api/worker/v1/status", ``, header) - assert.Equal(t, 404, response.StatusCode, "status mismatch") - - header = map[string]string{ - "x-rh-identity": "eyJlbnRpdGxlbWVudHMiOnsiaW5zaWdodHMiOnsiaXNfZW50aXRsZWQiOnRydWV9LCJzbWFydF9tYW5hZ2VtZW50Ijp7ImlzX2VudGl0bGVkIjp0cnVlfSwib3BlbnNoaWZ0Ijp7ImlzX2VudGl0bGVkIjp0cnVlfSwiaHlicmlkIjp7ImlzX2VudGl0bGVkIjp0cnVlfSwibWlncmF0aW9ucyI6eyJpc19lbnRpdGxlZCI6dHJ1ZX0sImFuc2libGUiOnsiaXNfZW50aXRsZWQiOnRydWV9fSwiaWRlbnRpdHkiOnsiYWNjb3VudF9udW1iZXIiOiIwMDAwMDAiLCJ0eXBlIjoiVXNlciIsInVzZXIiOnsidXNlcm5hbWUiOiJ1c2VyIiwiZW1haWwiOiJ1c2VyQHVzZXIudXNlciIsImZpcnN0X25hbWUiOiJ1c2VyIiwibGFzdF9uYW1lIjoidXNlciIsImlzX2FjdGl2ZSI6dHJ1ZSwiaXNfb3JnX2FkbWluIjp0cnVlLCJpc19pbnRlcm5hbCI6dHJ1ZSwibG9jYWxlIjoiZW4tVVMifSwiaW50ZXJuYWwiOnsib3JnX2lkIjoiMDAwMDAwIn19fQ==", - } - - response = test.SendHTTPWithHeader(handler, "GET", "/api/worker/v1/status", ``, header) - assert.Equal(t, 200, response.StatusCode, "status mismatch") -} - func TestOAuth(t *testing.T) { tempdir, err := ioutil.TempDir("", "worker-tests-") require.NoError(t, err) @@ -288,7 +252,7 @@ func TestOAuth(t *testing.T) { q, err := fsjobqueue.New(tempdir) require.NoError(t, err) - workerServer := worker.NewServer(nil, q, tempdir, []string{"000000"}) + workerServer := worker.NewServer(nil, q, tempdir) handler := workerServer.Handler() workSrv := httptest.NewServer(handler)