From 4a843ae26f7fdcc3333205873ac5ba757057928f Mon Sep 17 00:00:00 2001 From: tvpartytonight Date: Mon, 23 Oct 2023 16:56:31 -0700 Subject: [PATCH 1/2] (PUP-11973) WIP --- src/clj/puppetlabs/puppetserver/ringutils.clj | 6 ++++++ .../puppetlabs/services/master/master_core.clj | 15 ++++++++++----- .../puppetlabs/services/master/master_service.clj | 3 ++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/clj/puppetlabs/puppetserver/ringutils.clj b/src/clj/puppetlabs/puppetserver/ringutils.clj index afc64f3f2..7e69dcafd 100644 --- a/src/clj/puppetlabs/puppetserver/ringutils.clj +++ b/src/clj/puppetlabs/puppetserver/ringutils.clj @@ -75,6 +75,12 @@ (handler req) {:status 403 :body "Forbidden."}))) +(defn wrap-with-certname-as-compiler + "Function that returns middleware that add X-Puppet-Compiler-Name to the response" + [handler name] + (fn [request] + (ring/header (handler request) "X-Puppet-Compiler-Name" name))) + (defn wrap-with-puppet-version-header "Function that returns a middleware that adds an X-Puppet-Version header to the response." diff --git a/src/clj/puppetlabs/services/master/master_core.clj b/src/clj/puppetlabs/services/master/master_core.clj index 8aef797f9..6f42e6e4f 100644 --- a/src/clj/puppetlabs/services/master/master_core.clj +++ b/src/clj/puppetlabs/services/master/master_core.clj @@ -1313,14 +1313,16 @@ wrap-middleware :- IFn [handler :- IFn authorization-fn :- IFn - puppet-version :- schema/Str] + puppet-version :- schema/Str + certname :- schema/Str] (-> handler authorization-fn (middleware/wrap-uncaught-errors :plain) middleware/wrap-request-logging i18n/locale-negotiator middleware/wrap-response-logging - (ringutils/wrap-with-puppet-version-header puppet-version))) + (ringutils/wrap-with-puppet-version-header puppet-version) + (ringutils/wrap-with-certname-as-compiler certname))) (schema/defn ^:always-validate get-master-route-config "Get the webserver route configuration for the master service" @@ -1366,15 +1368,18 @@ environment-class-cache-enabled :- schema/Bool boltlib-path :- (schema/maybe [schema/Str]) bolt-builtin-content-dir :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + bolt-projects-dir :- (schema/maybe schema/Str) + certname :- schema/Str] (let [ruby-request-handler (wrap-middleware handle-request wrap-with-authorization-check - puppet-version) + puppet-version + certname) clojure-request-wrapper (fn [handler] (wrap-middleware (ring/wrap-params handler) wrap-with-authorization-check - puppet-version))] + puppet-version + certname))] (root-routes ruby-request-handler clojure-request-wrapper jruby-service diff --git a/src/clj/puppetlabs/services/master/master_service.clj b/src/clj/puppetlabs/services/master/master_service.clj index 6d9571bcf..8b879b86e 100644 --- a/src/clj/puppetlabs/services/master/master_service.clj +++ b/src/clj/puppetlabs/services/master/master_service.clj @@ -149,7 +149,8 @@ environment-class-cache-enabled boltlib-path bolt-builtin-content-dir - bolt-projects-dir)) + bolt-projects-dir + certname)) routes (comidi/context path ring-app) route-metadata (comidi/route-metadata routes) comidi-handler (comidi/routes->handler routes) From 467e1e29a8dec05f162c97bc50af22043af66cf2 Mon Sep 17 00:00:00 2001 From: tvpartytonight Date: Wed, 25 Oct 2023 08:42:07 -0700 Subject: [PATCH 2/2] (PUP-11973) Only send compiler header for v3 catalog requests --- src/clj/puppetlabs/puppetserver/ringutils.clj | 3 +- .../legacy_routes/legacy_routes_service.clj | 3 +- .../services/master/master_core.clj | 32 ++++++++++--------- .../services/master/master_service_test.clj | 11 +++++++ .../services/master/master_core_test.clj | 13 +++++--- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/clj/puppetlabs/puppetserver/ringutils.clj b/src/clj/puppetlabs/puppetserver/ringutils.clj index 7e69dcafd..8be06225e 100644 --- a/src/clj/puppetlabs/puppetserver/ringutils.clj +++ b/src/clj/puppetlabs/puppetserver/ringutils.clj @@ -76,7 +76,8 @@ {:status 403 :body "Forbidden."}))) (defn wrap-with-certname-as-compiler - "Function that returns middleware that add X-Puppet-Compiler-Name to the response" + "Function that returns middleware that add X-Puppet-Compiler-Name to the response, + only for the posts to the v3 catalog endpoint. Otherwise, do nothing." [handler name] (fn [request] (ring/header (handler request) "X-Puppet-Compiler-Name" name))) diff --git a/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj b/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj index 676a59656..32fcb8e20 100644 --- a/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj +++ b/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj @@ -45,7 +45,8 @@ false nil nil - nil)) + nil + (get-in config [:puppetserver :certname]))) master-route-handler (comidi/routes->handler master-routes) master-mount (master-core/get-master-mount master-ns diff --git a/src/clj/puppetlabs/services/master/master_core.clj b/src/clj/puppetlabs/services/master/master_core.clj index 6f42e6e4f..09d1e0eda 100644 --- a/src/clj/puppetlabs/services/master/master_core.clj +++ b/src/clj/puppetlabs/services/master/master_core.clj @@ -1051,7 +1051,8 @@ "v3 route tree for the ruby side of the master service." [request-handler :- IFn bolt-builtin-content-dir :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + bolt-projects-dir :- (schema/maybe schema/Str) + certname :- schema/Str] (comidi/routes (comidi/GET ["/node/" [#".*" :rest]] request (request-handler request)) @@ -1072,7 +1073,8 @@ (comidi/GET ["/catalog/" [#".*" :rest]] request (request-handler (assoc request :include-code-id? true))) (comidi/POST ["/catalog/" [#".*" :rest]] request - (request-handler (assoc request :include-code-id? true))) + (let [request-handler (ringutils/wrap-with-certname-as-compiler request-handler certname)] + (request-handler (assoc request :include-code-id? true)))) (comidi/PUT ["/facts/" [#".*" :rest]] request (request-handler request)) (comidi/PUT ["/report/" [#".*" :rest]] request @@ -1178,9 +1180,10 @@ wrap-with-jruby-queue-limit :- IFn boltlib-path :- (schema/maybe [schema/Str]) bolt-builtin-content-dir :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + bolt-projects-dir :- (schema/maybe schema/Str) + certname :- schema/Str] (comidi/context "/v3" - (v3-ruby-routes ruby-request-handler bolt-builtin-content-dir bolt-projects-dir) + (v3-ruby-routes ruby-request-handler bolt-builtin-content-dir bolt-projects-dir certname) (comidi/wrap-routes (v3-clojure-routes jruby-service get-code-content-fn @@ -1292,7 +1295,8 @@ environment-class-cache-enabled :- schema/Bool boltlib-path :- (schema/maybe [schema/Str]) bolt-builtin-content-dir :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + bolt-projects-dir :- (schema/maybe schema/Str) + certname :- schema/Str] (comidi/routes (v3-routes ruby-request-handler clojure-request-wrapper @@ -1303,7 +1307,8 @@ wrap-with-jruby-queue-limit boltlib-path bolt-builtin-content-dir - bolt-projects-dir) + bolt-projects-dir + certname) (v4-routes clojure-request-wrapper jruby-service wrap-with-jruby-queue-limit @@ -1313,16 +1318,14 @@ wrap-middleware :- IFn [handler :- IFn authorization-fn :- IFn - puppet-version :- schema/Str - certname :- schema/Str] + puppet-version :- schema/Str] (-> handler authorization-fn (middleware/wrap-uncaught-errors :plain) middleware/wrap-request-logging i18n/locale-negotiator middleware/wrap-response-logging - (ringutils/wrap-with-puppet-version-header puppet-version) - (ringutils/wrap-with-certname-as-compiler certname))) + (ringutils/wrap-with-puppet-version-header puppet-version))) (schema/defn ^:always-validate get-master-route-config "Get the webserver route configuration for the master service" @@ -1372,14 +1375,12 @@ certname :- schema/Str] (let [ruby-request-handler (wrap-middleware handle-request wrap-with-authorization-check - puppet-version - certname) + puppet-version) clojure-request-wrapper (fn [handler] (wrap-middleware (ring/wrap-params handler) wrap-with-authorization-check - puppet-version - certname))] + puppet-version))] (root-routes ruby-request-handler clojure-request-wrapper jruby-service @@ -1389,7 +1390,8 @@ environment-class-cache-enabled boltlib-path bolt-builtin-content-dir - bolt-projects-dir))) + bolt-projects-dir + certname))) (def MasterStatusV1 {(schema/optional-key :experimental) {:http-metrics [http-metrics/RouteSummary] diff --git a/test/integration/puppetlabs/services/master/master_service_test.clj b/test/integration/puppetlabs/services/master/master_service_test.clj index afc2def2c..50e5cbb16 100644 --- a/test/integration/puppetlabs/services/master/master_service_test.clj +++ b/test/integration/puppetlabs/services/master/master_service_test.clj @@ -694,6 +694,17 @@ (finally (jruby-testutils/return-instance jruby-service jruby-instance :http-report-processor-metrics-test))))))) +(deftest ^:integration compiler-name-as-header + (testing "POSTs to the v3 catalog endpoint return the certname as a header" + (bootstrap-testutils/with-puppetserver-running + app + {:jruby-puppet {:gem-path gem-path + :max-active-instances 1 + :server-code-dir test-resources-code-dir + :server-conf-dir master-service-test-runtime-dir}} + (let [resp (http-post "/puppet/v3/catalog/foo?environment=production" "")] + (is (= "localhost" (get-in resp [:headers "x-puppet-compiler-name"]))))))) + (deftest encoded-spaces-test (testing "Encoded spaces should be routed correctly" (bootstrap-testutils/with-puppetserver-running diff --git a/test/unit/puppetlabs/services/master/master_core_test.clj b/test/unit/puppetlabs/services/master/master_core_test.clj index ce25196b8..a52ca6712 100644 --- a/test/unit/puppetlabs/services/master/master_core_test.clj +++ b/test/unit/puppetlabs/services/master/master_core_test.clj @@ -51,7 +51,8 @@ true nil ["./dev-resources/puppetlabs/services/master/master_core_test/builtin_bolt_content"] - "./dev-resources/puppetlabs/services/master/master_core_test/bolt_projects") + "./dev-resources/puppetlabs/services/master/master_core_test/bolt_projects" + "test-certname") (comidi/routes->handler) (wrap-middleware identity puppet-version))) @@ -65,10 +66,12 @@ request (partial app-request app)] (is (= 200 (:status (request "/v3/environments")))) (is (= 200 (:status (request "/v3/catalog/bar?environment=environment1234")))) - (is (= 200 (:status (app (-> {:request-method :post - :uri "/v3/catalog/bar" - :content-type "application/x-www-form-urlencoded"} - (ring-mock/body "environment=environment1234")))))) + (let [response (app (-> {:request-method :post + :uri "/v3/catalog/bar" + :content-type "application/x-www-form-urlencoded"} + (ring-mock/body "environment=environment1234")))] + (is (= "test-certname" (get-in response [:headers "X-Puppet-Compiler-Name"])) + (is (= 200 (:status response))))) (is (nil? (request "/foo"))) (is (nil? (request "/foo/bar"))) (doseq [[method paths]