Uploaded image for project: 'Puppet Server'
  1. Puppet Server
  2. SERVER-589

Update "content-type" references to work for new Ring version

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Normal
    • Resolution: Won't Do
    • Affects Version/s: None
    • Fix Version/s: SERVER 1.y, SERVER 2.y
    • Component/s: None
    • Labels:
    • Template:

      Description

      As part of SERVER-544, we upgraded Puppet Server's dependency on ring-core to 1.3.2. In the newer ring versions, the content-type header for a request is only managed through the :headers map in the ring request rather than as a top-level element in the request - see https://github.com/ring-clojure/ring/commit/bdffd464d2e803ae73707e46eb434e6b07ef0d21. Because we have some code paths in Puppet Server that manage content-type via the top-level element in the request but didn't necessarily want to take on the work of updating those paths to work with the new ring-core as-is, we ended up duplicating some code from ring-core and updating it to be backward-compatible in its handling of content-type - see https://github.com/puppetlabs/puppet-server/blob/d623c77a0c9d881ebf59a2fef26fe543a1300120/src/clj/puppetlabs/puppetserver/ring/middleware/params.clj#L40-L42.

      IMHO, the changes in Ring are good. Otherwise, we're left with the potential ambiguous situation where if content-type were represented in two different places in the request but had different values, it isn't clear which one to honor.
      As a step toward being able to eliminate this code duplication from Ring, we should update Puppet Server to work with the new behavior for content-type.

      One particular path through Puppet Server that will likely be affected by this is the work that we did to support the "file_bucket_file" endpoint. See:

      https://github.com/puppetlabs/puppet-server/blob/d623c77a0c9d881ebf59a2fef26fe543a1300120/src/clj/puppetlabs/services/master/master_core.clj#L47-L50

      For that case, we ended up being dependent upon setting the content-type at the top level of the request to octet-stream but letting the corresponding header map value stay as text/plain if it was passed in that way. That was done so that a legacy Puppet master would accept the binary content in the request. We'll probably need to rework that code, though, to work with the new Ring implementation.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              Unassigned
              Reporter:
              jeremy.barlow Jeremy Barlow
              QA Contact:
              Erik Dasher
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Zendesk Support