Skip to content

Commit

Permalink
(PUP-11973) Only send compiler header for v3 catalog requests
Browse files Browse the repository at this point in the history
  • Loading branch information
tvpartytonight committed Nov 15, 2023
1 parent 4a843ae commit 467e1e2
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 22 deletions.
3 changes: 2 additions & 1 deletion src/clj/puppetlabs/puppetserver/ringutils.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 17 additions & 15 deletions src/clj/puppetlabs/services/master/master_core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions test/unit/puppetlabs/services/master/master_core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Expand All @@ -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]
Expand Down

0 comments on commit 467e1e2

Please sign in to comment.