From b731518daf88cf1bdd2408ab8f4f4beb65f21d2d Mon Sep 17 00:00:00 2001 From: "Sean P. McDonald" Date: Wed, 25 Sep 2024 12:30:02 -0500 Subject: [PATCH] (PE-39250) Remove project APIs and configuration Removes access to file_metadata and compile options which allowed fetching data and compiling for bolt projects. This commit does not remove all project based code, it is restricted to covering enough ground to remove access to any functionality, but there may still be some internal code left over. --- .../legacy_routes/legacy_routes_service.clj | 2 - .../services/master/master_core.clj | 58 ++----- .../services/master/master_service.clj | 4 - .../puppetserver-lib/puppet/server/master.rb | 2 +- .../services/master/master_service_test.clj | 152 ------------------ .../services/master/master_core_test.clj | 29 +--- 6 files changed, 16 insertions(+), 231 deletions(-) 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 270cffaf5..5a5dd3043 100644 --- a/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj +++ b/src/clj/puppetlabs/services/legacy_routes/legacy_routes_service.clj @@ -44,8 +44,6 @@ (constantly nil) false nil - nil - nil (get-in config [:puppetserver :certname]))) master-route-handler (comidi/routes->handler master-routes) master-mount (master-core/get-master-mount diff --git a/src/clj/puppetlabs/services/master/master_core.clj b/src/clj/puppetlabs/services/master/master_core.clj index 5dfea3ae6..1b6de7eb9 100644 --- a/src/clj/puppetlabs/services/master/master_core.clj +++ b/src/clj/puppetlabs/services/master/master_core.clj @@ -982,36 +982,23 @@ (schema/defn ^:always-validate compile-fn :- IFn [jruby-service :- (schema/protocol jruby-protocol/JRubyPuppetService) current-code-id-fn :- IFn - boltlib-path :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + boltlib-path :- (schema/maybe [schema/Str])] (fn [request] (let [request-options (-> request :body slurp (validated-body CompileRequest)) - versioned-project (get request-options "versioned_project") environment (get request-options "environment")] - ;; Check to ensure environment/versioned_project are mutually exlusive and - ;; at least one of them is set. - (cond - (and versioned-project environment) - {:status 400 - :headers {"Content-Type" "text/plain"} - :body (i18n/tru "A compile request cannot specify both `environment` and `versioned_project` parameters.")} - - (and (nil? versioned-project) (nil? environment)) + ;; Check to ensure environment is set. + + (if (nil? environment) {:status 400 :headers {"Content-Type" "text/plain"} :body (i18n/tru "A compile request must include an `environment` or `versioned_project` parameter.")} - :else - (let [compile-options (if versioned-project - ;; we need to parse some data from the project config for project compiles - (parse-project-compile-data request-options versioned-project bolt-projects-dir) - ;; environment compiles only need to set the code ID - (assoc request-options + (let [compile-options (assoc request-options "code_id" - (current-code-id-fn environment)))] + (current-code-id-fn environment))] {:status 200 :headers {"Content-Type" "application/json"} :body (json/encode @@ -1035,9 +1022,8 @@ [jruby-service :- (schema/protocol jruby-protocol/JRubyPuppetService) wrap-with-jruby-queue-limit :- IFn current-code-id-fn :- IFn - boltlib-path :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] - (-> (compile-fn jruby-service current-code-id-fn boltlib-path bolt-projects-dir) + boltlib-path :- (schema/maybe [schema/Str])] + (-> (compile-fn jruby-service current-code-id-fn boltlib-path) (jruby-request/wrap-with-jruby-instance jruby-service) wrap-with-jruby-queue-limit jruby-request/wrap-with-error-handling)) @@ -1061,17 +1047,14 @@ v3-ruby-routes :- bidi-schema/RoutePair "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) certname :- schema/Str] (comidi/routes (comidi/GET ["/node/" [#".*" :rest]] request (request-handler request)) (comidi/GET ["/file_content/" [#".*" :rest]] request - ;; Not strictly ruby routes anymore because of this - (file-serving/file-content-handler bolt-builtin-content-dir bolt-projects-dir request-handler (ring/params-request request))) + (request-handler request)) (comidi/GET ["/file_metadatas/" [#".*" :rest]] request - (file-serving/file-metadatas-handler bolt-builtin-content-dir bolt-projects-dir request-handler (ring/params-request request))) + (request-handler request)) (comidi/GET ["/file_metadata/" [#".*" :rest]] request (request-handler request)) (comidi/GET ["/file_bucket_file/" [#".*" :rest]] request @@ -1101,8 +1084,7 @@ current-code-id-fn :- IFn cache-enabled :- schema/Bool wrap-with-jruby-queue-limit :- IFn - boltlib-path :- (schema/maybe [schema/Str]) - bolt-projects-dir :- (schema/maybe schema/Str)] + boltlib-path :- (schema/maybe [schema/Str])] (let [class-handler (create-cacheable-info-handler-with-middleware (fn [jruby env] (some-> jruby-service @@ -1137,8 +1119,7 @@ jruby-service wrap-with-jruby-queue-limit current-code-id-fn - boltlib-path - bolt-projects-dir)] + boltlib-path)] (comidi/routes (comidi/POST "/compile" request (compile-handler' request)) @@ -1190,19 +1171,16 @@ environment-class-cache-enabled :- schema/Bool 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) certname :- schema/Str] (comidi/context "/v3" - (v3-ruby-routes ruby-request-handler bolt-builtin-content-dir bolt-projects-dir certname) + (v3-ruby-routes ruby-request-handler certname) (comidi/wrap-routes (v3-clojure-routes jruby-service get-code-content-fn current-code-id-fn environment-class-cache-enabled wrap-with-jruby-queue-limit - boltlib-path - bolt-projects-dir) + boltlib-path) clojure-request-wrapper))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -1305,8 +1283,6 @@ current-code-id-fn :- IFn 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) certname :- schema/Str] (comidi/routes (v3-routes ruby-request-handler @@ -1317,8 +1293,6 @@ environment-class-cache-enabled wrap-with-jruby-queue-limit boltlib-path - bolt-builtin-content-dir - bolt-projects-dir certname) (v4-routes clojure-request-wrapper jruby-service @@ -1381,8 +1355,6 @@ wrap-with-jruby-queue-limit :- IFn 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) certname :- schema/Str] (let [ruby-request-handler (wrap-middleware handle-request wrap-with-authorization-check @@ -1400,8 +1372,6 @@ current-code-id environment-class-cache-enabled boltlib-path - bolt-builtin-content-dir - bolt-projects-dir certname))) (def MasterStatusV1 diff --git a/src/clj/puppetlabs/services/master/master_service.clj b/src/clj/puppetlabs/services/master/master_service.clj index 1c332f100..e97563fc3 100644 --- a/src/clj/puppetlabs/services/master/master_service.clj +++ b/src/clj/puppetlabs/services/master/master_service.clj @@ -115,8 +115,6 @@ certname (get-in config [:puppetserver :certname]) localcacert (get-in config [:puppetserver :localcacert]) puppet-version (get-in config [:puppetserver :puppet-version]) - bolt-builtin-content-dir (get-in config [:bolt :builtin-content-dir] []) - bolt-projects-dir (get-in config [:bolt :projects-dir]) max-queued-requests (get-in config [:jruby-puppet :max-queued-requests] 0) max-retry-delay (get-in config [:jruby-puppet :max-retry-delay] 1800) settings (ca/config->master-settings config) @@ -147,8 +145,6 @@ wrap-with-jruby-queue-limit environment-class-cache-enabled boltlib-path - bolt-builtin-content-dir - bolt-projects-dir certname)) routes (comidi/context path ring-app) route-metadata (comidi/route-metadata routes) diff --git a/src/ruby/puppetserver-lib/puppet/server/master.rb b/src/ruby/puppetserver-lib/puppet/server/master.rb index 9414b7ae9..d9a18188a 100644 --- a/src/ruby/puppetserver-lib/puppet/server/master.rb +++ b/src/ruby/puppetserver-lib/puppet/server/master.rb @@ -248,7 +248,7 @@ def check_cadir_for_deprecation_warning # Each array element is examined, if it is expected to be a map # we call back to the convert_java_args_to_ruby method, if it # is expected to be an array, we recurse otherwise we do not modify - # the value. + # the value. def resolve_java_objects_from_list(list) list.map do |value| if value.kind_of?(Java::ClojureLang::IPersistentMap) diff --git a/test/integration/puppetlabs/services/master/master_service_test.clj b/test/integration/puppetlabs/services/master/master_service_test.clj index 257e9c1c9..b45f50ce1 100644 --- a/test/integration/puppetlabs/services/master/master_service_test.clj +++ b/test/integration/puppetlabs/services/master/master_service_test.clj @@ -770,155 +770,3 @@ Integer/parseInt)] (is (= 503 status-code)) (is (<= 0 retry-after 1800)))))))) - -(deftest ^:integration project-file-content - (bootstrap-testutils/with-puppetserver-running - app - {:bolt {:builtin-content-dir ["./dev-resources/puppetlabs/services/master/master_core_test/builtin_bolt_content"] - :projects-dir "./dev-resources/puppetlabs/services/master/master_core_test/bolt_projects"} - :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}} - - (testing "can retrieve file_content from the modules mount from a project" - (let [response (http-get "/puppet/v3/file_content/modules/utilities/etc/greeting?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "Good morning\n" (:body response))) - (is (= "13" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - ;; this also tests that we can retrieve file content from the .modules mount of the project - ;; since that is where this task is located - (testing "can retrieve file_content from the tasks mount from a project" - (let [response (http-get "/puppet/v3/file_content/tasks/utilities/blah?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "bye" (:body response))) - (is (= "3" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve file_content from the top level of a project" - (let [response (http-get "/puppet/v3/file_content/tasks/local/init.sh?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= ". $PT__installdir/helpers/files/marco.sh\nmarco\n" (:body response))) - (is (= "63" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve file_content from a project with an embedded structure" - (let [response (http-get "/puppet/v3/file_content/modules/test/packages?versioned_project=embedded_e19e09")] - (is (= 200 (:status response))) - (is (= "vim" (:body response))) - (is (= "3" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve file_content from a custom modulepath" - (let [response (http-get "/puppet/v3/file_content/modules/helpers/marco.sh?versioned_project=local_afafaf")] - (is (= 200 (:status response))) - (is (= "marco () {\n echo \"polo\"\n}\n" (:body response))) - (is (= "27" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve built-in file_content" - (let [response (http-get "/puppet/v3/file_content/tasks/bic_module_one/init.sh?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= ". $PT__installdir/helpers/files/marco.sh\nmarco\n" (:body response))) - (is (= "63" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve overriden built-in file_content" - (let [response (http-get "/puppet/v3/file_content/tasks/bic_module_one/init.sh?versioned_project=override_builtin_content")] - (is (= 200 (:status response))) - (is (= ". $PT__installdir/helpers/files/marco.sh\noverride_marco\n" (:body response))) - (is (= "73" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "cannot retrieve file_content from the default modulepath when a custom modulepath is set" - (let [response (http-get "/puppet/v3/file_content/tasks/utilities/blah?versioned_project=local_afafaf")] - (is (= 404 (:status response))))) - - (testing "mount point not found" - (let [response (http-get "/puppet/v3/file_content/glorb/test/packages?versioned_project=local_23")] - (is (= 404 (:status response))))) - - (testing "project not found" - (let [response (http-get "/puppet/v3/file_content/modules/test/packages?versioned_project=nothere")] - (is (= 404 (:status response))))) - - (testing "module not found" - (let [response (http-get "/puppet/v3/file_content/modules/nomodule/packages?versioned_project=embedded_e19e09")] - (is (= 404 (:status response))))) - - (testing "missing path?" - (let [response (http-get "/puppet/v3/file_content/modules/test/?versioned_project=embedded_e19e09")] - (is (= 404 (:status response))))) - - (testing "can retrieve plugin metadata" - (let [response (http-get "/puppet/v3/file_metadatas/plugins?versioned_project=local_23") - [file-entry] (filter #(= "puppet/monkey_patch.rb" (get % "relative_path")) (json/decode (:body response)))] - ;; Only check some of the entries that won't vary based on the test environment - (is (= nil (get file-entry "destination"))) - (is (= "file" (get file-entry "type"))) - (is (= "sha256" (get-in file-entry ["checksum" "type"]))) - (is (= "{sha256}76b2e03b82880885385595045033c4e3122e373c7023e037461a650ec85829ad" (get-in file-entry ["checksum" "value"]))) - ;; Does it choose from the right module? - (is (str/ends-with? (get file-entry "path") "modules/helpers/lib")))) - - (testing "can retrieve builtin plugin metadata" - (let [response (http-get "/puppet/v3/file_metadatas/plugins?versioned_project=local_23") - [file-entry] (filter #(= "puppet/builtin_monkey_patch.rb" (get % "relative_path")) (json/decode (:body response)))] - ;; Only check some of the entries that won't vary based on the test environment - (is (= nil (get file-entry "destination"))) - (is (= "file" (get file-entry "type"))) - (is (= "sha256" (get-in file-entry ["checksum" "type"]))) - (is (= "{sha256}76b2e03b82880885385595045033c4e3122e373c7023e037461a650ec85829ad" (get-in file-entry ["checksum" "value"]))) - ;; Does it choose from the right module? - (is (str/ends-with? (get file-entry "path") "bic_module_one/lib")))) - - (testing "can retrieve plugin files" - (let [response (http-get "/puppet/v3/file_content/plugins/puppet/monkey_patch.rb?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "class NilClass\n def empty?\n true\n end\nend\n" (:body response))) - (is (= "59" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve builtin plugin files" - (let [response (http-get "/puppet/v3/file_content/plugins/puppet/builtin_monkey_patch.rb?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "class NilClass\n def empty?\n true\n end\nend\n" (:body response))) - (is (= "59" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve overridden builtin plugin files" - (let [response (http-get "/puppet/v3/file_content/plugins/puppet/builtin_monkey_patch.rb?versioned_project=override_builtin_content")] - (is (= 200 (:status response))) - (is (= "overridden_class NilClass\n def empty?\n true\n end\nend\n" (:body response))) - (is (= "70" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve plugin files from the top level project lib dir" - (let [response (http-get "/puppet/v3/file_content/plugins/puppet/comment.rb?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "# This is the project\n" (:body response))) - (is (= "22" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "can retrieve pluginfacts metadata" - (let [response (http-get "/puppet/v3/file_metadatas/pluginfacts?versioned_project=local_23") - [file-entry] (filter #(= "something" (get % "relative_path")) (json/decode (:body response)))] - (is (= nil (get file-entry "destination"))) - (is (= "file" (get file-entry "type"))) - (is (= "sha256" (get-in file-entry ["checksum" "type"]))) - (is (= "{sha256}4bc453b53cb3d914b45f4b250294236adba2c0e09ff6f03793949e7e39fd4cc1" (get-in file-entry ["checksum" "value"]))))) - - (testing "can retrieve pluginfacts files" - (let [response (http-get "/puppet/v3/file_content/pluginfacts/unhelpful?versioned_project=local_23")] - (is (= 200 (:status response))) - (is (= "factually unhelpful\n" (:body response))) - (is (= "20" (get-in response [:headers "content-length"]))) - (is (= "application/octet-stream" (get-in response [:headers "content-type"]))))) - - (testing "doesn't support nonstandard options" - (let [response (http-get "/puppet/v3/file_metadatas/plugins?versioned_project=local_23&links=manage")] - (is (= 400 (:status response))) - (is (= "Not all parameter values are supported in this implementation: \nThe only supported value of `links` at this time is `follow`" - (:body response))))))) diff --git a/test/unit/puppetlabs/services/master/master_core_test.clj b/test/unit/puppetlabs/services/master/master_core_test.clj index 9386820c0..7b33895bc 100644 --- a/test/unit/puppetlabs/services/master/master_core_test.clj +++ b/test/unit/puppetlabs/services/master/master_core_test.clj @@ -50,8 +50,6 @@ (constantly nil) true nil - ["./dev-resources/puppetlabs/services/master/master_core_test/builtin_bolt_content"] - "./dev-resources/puppetlabs/services/master/master_core_test/bolt_projects" "test-certname") (comidi/routes->handler) (wrap-middleware identity puppet-version))) @@ -447,36 +445,11 @@ :trusted_facts {:values {}} :variables {:values {}}}))))] (is (= 200 (:status response))))) - (testing "compile endpoint for projects" + (testing "compile endpoint fails with no environment" (let [response (app (-> {:request-method :post :uri "/v3/compile" :content-type "application/json"} (ring-mock/body (json/encode {:certname "foo" - :versioned_project "fake_project" - :code_ast "{\"__pcore_something\": \"Foo\"}" - :facts {:values {}} - :trusted_facts {:values {}} - :variables {:values {}} - :options {:compile_for_plan true}}))))] - (is (= 200 (:status response))))) - (testing "compile endpoint fails with no environment or versioned_project" - (let [response (app (-> {:request-method :post - :uri "/v3/compile" - :content-type "application/json"} - (ring-mock/body (json/encode {:certname "foo" - :code_ast "{\"__pcore_something\": \"Foo\"}" - :facts {:values {}} - :trusted_facts {:values {}} - :variables {:values {}} - :options {:compile_for_plan true}}))))] - (is (= 400 (:status response))))) - (testing "compile endpoint fails with both environment and versioned_project" - (let [response (app (-> {:request-method :post - :uri "/v3/compile" - :content-type "application/json"} - (ring-mock/body (json/encode {:certname "foo" - :environment "production" - :versioned_project "fake_project" :code_ast "{\"__pcore_something\": \"Foo\"}" :facts {:values {}} :trusted_facts {:values {}}