Skip to content

Commit

Permalink
Make deployment atomic
Browse files Browse the repository at this point in the history
Deployments aren't currently atomic, so if a deploy is interrupted, it
can leave the repo in an inconsistent state.

This adds the basics of atomic deploys, with the following additional
changes:

* an update to pomegranate (0.0.13 -> 0.3.0) - I'm using aether to
  deploy from the tmp upload dir to the repo, and need the :artifact-map
  functionality that doesn't exist in 0.0.13
* replacing the auth helper macros with functions - this is to make it
  easier to debug, and easier to determine where values come
  from (instead of doing magic binding, like we were doing with
  'account)

The atomic deploy functionality uses sessions (since aether honors
session cookies) to store uploads in a tmp dir that is scoped to the
deploy. Once we see a non-snapshot maven-metadata.xml, we finalize the
deployment by verifying the contents and then deploying them to the
actual repo. This process ignores checksum files, since the redeploy
recreates them.

The finalize process also validates the deploy, which includes the
existing validations (that were once per-artifact), plus adds some new
ones that operate over the full set of artifacts. The validations
are (existing validations marked with ^):

* verify a pom was uploaded
* verify the pom parses
* verify the group, name, and version of a valid format ^
* verify the gav values in the pom match the gav in the url
* verify this isn't a redeploy of a non-SNAPSHOT version ^
* verify that a jar was uploaded if the pom packaging is jar
* verify that the provided checksums match the artifacts
* verify that if any signature is uploaded, then every artifact has a
  signature

Now that we are validating after all the artifacts are pushed, we no
longer fail-fast on redeploys or invalid gav's, but I don't think that
should cause issues.

The change to require valid poms is debatable, given that it will
prevent deploys of projects that are affected by #233.
  • Loading branch information
tobias committed Feb 22, 2016
1 parent c611fbc commit 57fc478
Show file tree
Hide file tree
Showing 11 changed files with 378 additions and 180 deletions.
2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[org.apache.maven/maven-model "3.0.4"
:exclusions
[org.codehaus.plexus/plexus-utils]]
[com.cemerick/pomegranate "0.0.13"
[com.cemerick/pomegranate "0.3.0"
:exclusions
[org.apache.httpcomponents/httpcore
commons-logging]]
Expand Down
20 changes: 9 additions & 11 deletions src/clojars/auth.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@
(:require [cemerick.friend :as friend]
[clojars.db :refer [group-membernames]]))

(defmacro with-account [body]
`(friend/authenticated (try-account ~body)))
(defn try-account [f]
(f (:username (friend/current-authentication))))

(defmacro try-account [body]
`(let [~'account (:username (friend/current-authentication))]
~body))
(defn with-account [f]
(friend/authenticated (try-account f)))

(defn authorized? [db account group]
(if account
(let [names (group-membernames db group)]
(or (some #{account} names) (empty? names)))))

(defmacro require-authorization [db group & body]
`(if (authorized? ~db ~'account ~group)
(do ~@body)
(friend/throw-unauthorized friend/*identity*
{:cemerick.friend/exprs (quote [~@body])
:cemerick.friend/required-roles ~group})))
(defn require-authorization [db account group f]
(if (authorized? db account group)
(f)
(friend/throw-unauthorized friend/*identity*
{:cemerick.friend/required-roles group})))
29 changes: 17 additions & 12 deletions src/clojars/file_utils.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
[file type]
(let [file' (io/file file)]
(io/file (.getParentFile file')
(format "%s.%s" (.getName file') (name type)))))
(format "%s.%s" (.getName file') (name type)))))

(defn- create-sum [f file type]
(let [file' (io/file file)]
Expand Down Expand Up @@ -35,17 +35,22 @@

(defn valid-sum?
"Checks to see if a sum of type `type` exists and is valid for `file`"
[file type]
(let [sig-file (sum-file file type)]
(and (.exists sig-file)
(= ((sum-generators type) (io/file file))
(slurp sig-file)))))
([file type]
(valid-sum? file type true))
([file type fail-if-missing?]
(let [sig-file (sum-file file type)]
(if (.exists sig-file)
(= ((sum-generators type) (io/file file))
(slurp sig-file))
(not fail-if-missing?)))))

(defn valid-sums?
"Checks to see if both md5 and sha1 sums exist and are valid for `file`"
[file]
(reduce (fn [valid? sig-type]
(and valid?
(valid-sum? file sig-type)))
true
[:md5 :sha1]))
([file]
(valid-sums? file true))
([file fail-if-missing?]
(reduce (fn [valid? sig-type]
(and valid?
(valid-sum? file sig-type fail-if-missing?)))
true
[:md5 :sha1])))
1 change: 1 addition & 0 deletions src/clojars/maven.clj
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
:licenses (mapv license-to-seq (.getLicenses model))
:scm (scm-to-map (.getScm model))
:authors (mapv #(.getName %) (.getContributors model))
:packaging (keyword (.getPackaging model))
:dependencies (mapv
(fn [d] {:group_name (.getGroupId d)
:jar_name (.getArtifactId d)
Expand Down
29 changes: 12 additions & 17 deletions src/clojars/routes/artifact.clj
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,26 @@
(defn show [db reporter stats group-id artifact-id]
(if-let [artifact (db/find-jar db group-id artifact-id)]
(auth/try-account
(view/show-jar db
reporter
stats
account
artifact
(db/recent-versions db group-id artifact-id 5)
(db/count-versions db group-id artifact-id)))))
#(view/show-jar db
reporter
stats
%
artifact
(db/recent-versions db group-id artifact-id 5)
(db/count-versions db group-id artifact-id)))))

(defn list-versions [db group-id artifact-id]
(if-let [artifact (db/find-jar db group-id artifact-id)]
(auth/try-account
(view/show-versions account
artifact
(db/recent-versions db group-id artifact-id)))))
#(view/show-versions % artifact
(db/recent-versions db group-id artifact-id)))))

(defn show-version [db reporter stats group-id artifact-id version]
(if-let [artifact (db/find-jar db group-id artifact-id version)]
(auth/try-account
(view/show-jar db
reporter
stats
account
artifact
(db/recent-versions db group-id artifact-id 5)
(db/count-versions db group-id artifact-id)))))
#(view/show-jar db reporter stats % artifact
(db/recent-versions db group-id artifact-id 5)
(db/count-versions db group-id artifact-id)))))

(defn response-based-on-format
"render appropriate response based on the file type suffix provided:
Expand Down
34 changes: 18 additions & 16 deletions src/clojars/routes/group.clj
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,24 @@
(GET ["/groups/:groupname", :groupname #"[^/]+"] [groupname]
(if-let [membernames (seq (db/group-membernames db groupname))]
(auth/try-account
(view/show-group db account groupname membernames))))
#(view/show-group db % groupname membernames))))
(POST ["/groups/:groupname", :groupname #"[^/]+"] [groupname username]
(if-let [membernames (seq (db/group-membernames db groupname))]
(auth/try-account
(auth/require-authorization
db
groupname
(cond
(some #{username} membernames)
(view/show-group db account groupname membernames
"They're already a member!")
(db/find-user db username)
(do (db/add-member db groupname username account)
(view/show-group db account groupname
(conj membernames username)))
:else
(view/show-group db account groupname membernames
(str "No such user: "
username)))))))))
(fn [account]
(auth/require-authorization
db
account
groupname
#(cond
(some #{username} membernames)
(view/show-group db account groupname membernames
"They're already a member!")
(db/find-user db username)
(do (db/add-member db groupname username account)
(view/show-group db account groupname
(conj membernames username)))
:else
(view/show-group db account groupname membernames
(str "No such user: "
username))))))))))
Loading

0 comments on commit 57fc478

Please sign in to comment.