zalando/friboo

Utility library for writing microservices in Clojure, with support for Swagger and OAuth

friboo's Issues

Something's not right with 1.4.0 artifact

Reproducible by:

• Delete ~/.m2/repository to wipe maven cache
• lein new app wtf
• Put this in wtf.core-test:

(ns wtf.core-test
  (:require [clojure.test :refer :all]
            [wtf.core :refer :all]
            [clj-time.core :as t]
            [amazonica.aws.s3 :as s3]
            [org.zalando.stups.friboo.system.audit-log :as audit]))

(def mock-log
  {:status 200
   :logged-on (t/now)
   :tokeninfo {"uid" "test"}})

(deftest test-audit-log
  (with-redefs [s3/put-object (constantly "OK")]
    (audit/store-audit-logs! (ref [mock-log]) "bucket")))

Yields:

lein test wtf.core-test
WARN  [main] o.z.s.f.s.audit-log - Could not store audit logs because of ["clojure.lang.ArityException: Wrong number of args (1) passed to: format/unparse"].

Ran 1 tests containing 0 assertions.
0 failures, 0 errors.

This should not happen. If you take a look at store-audit-logs! you'll see that the line containing unparse wasn’t modified since July. Also the line is hit by unit tests without problems.

Maybe just rebuild and rerelease?

Log warning if handler function returns no status code

Improvement, additional validation. If handler returns empty response (e.g. nil) no response status is available for statistic's metrics. This causes confusing statistic's metrics like 'some.id..POST' while ''some.id.200.POST'' is expected. I suggest log a warning in this case to help with diagnostics.

Probably this can be implemented as separate middleware but I think simplest ways is just to check this directly in add-metrics-filter inside (swap! status (fn [_] (.getStatus (cast HttpServletResponse response))))

Support alternate OAuth2 token passing via "Authorization: Token <token>" and "Authorization: Basic <base64>"

Looks like a hack, but is needed to support Docker client auth in Pier One

Log modifying REST calls to S3 bucket

Optional feature for HTTP servers:

All successfully executed, modifying HTTP requests (PUT, POST, PATCH, DELETE, ....) should be logged to an S3 bucket.

The bucket name, file prefix and flush interval should be configurable by environment variables.

Need to dissoc "open_id" key from tokeninfo from audit logs

here: https://github.com/zalando-stups/friboo/blob/fbfedf39dea957c6f2b51c58d4bb2d4a9ad26b55/src/org/zalando/stups/friboo/system/audit_log.clj#L69
"open_id" key contains JWT tokens and should not be logged

Access-Control-Allow-Headers does not accept wildcards

Relevant StackOverflow.

GZIP responses by default

Less time for data transfer = better.

Security for discovery and definition endpoints

It should be possible to protect the discovery and api definition endpoints (e.g. with OAuth2) because they might contain sensitive domain information.

Tune Jetty to not add "Date" header and remove Jetty version

We should tune the Jetty adapter to remove the "Date" HTTP header and the Jetty version.

Provide dependencies as a map

A suggestion:
Instead of providing dependencies as independent parameters:

(defn get-status [params request db tokens]
  ...)

Provide them as a map:

(defn get-status [params request {:keys [db tokens] :as deps}]
  ...)

Not all the dependencies are relevant for every handler function, having to maintain dependency additions and removals is annoying.
Order of dependencies does not matter.

Use namespaced keywords for system map components

In the org.zalando.stups.friboo.system/http-system-map some components are provided by default. They use keyword names that can collide with components defined by the framework user.
It would make sense to use less popular names (:org.zalando.stups.friboo.system/api instead of :api) for them.

Send HSTS headers everywhere

Make middleware stack customizable

Currently it is statically defined:
https://github.com/zalando-stups/friboo/blob/fbfedf39dea957c6f2b51c58d4bb2d4a9ad26b55/src/org/zalando/stups/friboo/system/http.clj#L162
Instead, we can introduce customization points, so that a user can add his own middleware to the existing stack.

Unzip Clair messages

Look at CLAIR.CONTENTTYPE MessageAttribute, if it's "application/base64gzip", then unbase64 and ungzip the Message.

Wrong json serialization of java.sql.Timestamp

Currently java.sql.Timestamps get serialized to

• 🚫 "2015-09-08T08:48:32.000+0200",

but according to RFC3339 it should be

• ✅ "2015-09-08T08:48:32.000+02:00", or
• ✅ "2015-09-08T08:48:32.000Z"

Upgrade Hystrix Dashboard resources

The current Hystrix Dashboard resources are not up-to-date with the included jar dependency. There were changes that broke the view.

Sensitive information should be masked out when logging the configuration

Sensitive information such as passwords, keys or other credentials should be "masked out" when logging configuration data (e.g. https://github.com/zalando-stups/friboo/blob/master/src/org/zalando/stups/friboo/config.clj#L46). Even when logging only with DEBUG level, it might be better to be on the safe side (any "temporary enabled" log will appear in a central logging service).

Proposed solution:

(defn- is-sensitive-key [k]
  (or (.contains (name k) "pass") (.contains (name k) "private")))

(defn mask [config]
  "Mask sensitive information such as passwords"
  (into {} (for [[k v] config] [k (if (is-sensitive-key k) "MASKED" v)])))

Pull apart HTTP and API

Currently notions of HTTP serving and API handling are complected together in the def-http-component and http-system-map functions. Clearly, there should be separate components:

• Http component takes care of serving HTTP requests on the configured port. Uses configuration with :http- prefix.
• Api component takes care of calling the corresponding handler functions according to the API spec. Uses application-specific configuration with :api- prefix.

Include leiningen template as a part of the repo

The template is here:
https://github.com/dryewo/friboo-template/

Consider adding a macro for defining db commands

(defmacro defquery-hystrix-commands [filename]
  `(do
     (defqueries ~filename)
     (generate-hystrix-commands)))

(defquery-hystrix-commands "db/queries.sql")

Docker image building doesn't work out of the box when using project template

There's a problem in building docker image when using project template:

• lein-scm-source creates scm-source.json in target dir. In project.clj target-path is set to target/%s so the location of the file is something like target/base+system+user+dev/scm-source.json
• Dockerfile expects scm-source.json to be in the root of the project

I guess the obvious fix is to change lein-scm-source to generate in project root instead, or make it configurable and preconfigure it in the template. I am happy to provide a PR if you think this is the correct fix.

Check service users in Kio not in services api

So applications, which belong to virtual teams, are not recognized as being owned by the 'main' team.

https://github.com/zalando-stups/friboo/blob/master/src/org/zalando/stups/friboo/user.clj#L71

Fix a typo in cron.clj

(at/stop-and-reset-pool! (:pool this#) :kill)

should be:

(at/stop-and-reset-pool! (:pool this#) :strategy :kill)

here: https://github.com/zalando-stups/friboo/blob/4c7d20fccb0fbaab03876360306edcc33b7088c1/src/org/zalando/stups/friboo/system/cron.clj#L99

require-service-team does not extract access token correctly

Update maintainers file, add contributing.md file

If @dryewo and @prayerslayer are now the project maintainers, let's change. :) I'm happy to add the contributing file, just let me know if you have a preference (file issue first suggesting change, or just file PR w/o prior notice--I'd go with the former approach to minimize unwanted noise).

Upgrade to swagger1st 0.16.0

data.json is gone.

Automate documentation generation

After #91.

During each normal Travis build (when TRAVIS_PULL_REQUEST is false), generate project documentation and publish it with Github Pages under the corresponding branch subfolder.

OAuth token refresher should not require access token URL environment variable

See here. This makes local testing hard. The underlying tokens library allows to set the tokens to use with an environment variable. Friboo should respect this 😄

More flexible audit logging

Hi there!

I'm not completely happy with the audit logging in friboo. The way it currently works is storing all successful (2xx) responses to PUT/POST/PATCH/DELETE requests, which is very unflexible and low-level.

Sometimes your audit logs involve data that's not in a HTTP body, you may want to ship it somewhere else than S3 (file, database, REST API...) and so forth.

I don't have a good idea yet how to do this in a good and simple way. Maybe one could (mis?)use timbre to have already an ecosystem for appenders available and/or easily plug custom ones?

Do not use undocumented Clojure features in template project

Generated project.clj uses undocumented #= reader macro. I would suggest to use enclosing let instead, use hard-code some value or, better yet, make it a template's parameter (since current code makes too many assumptions about environment).

let version would look like

(let [docker-image-name (some-> (str (System/getenv "DEFAULT_DOCKER_REGISTRY") "/")
                                                     "example_team/maratus"))]
  (defproject ...
    ))

BTW, why (str "/") is needed in generated project.clj file?

Use "TOKENINFO_URL" environment variable as fallback

We decided to standardize the OAuth 2 token info URL configuration: we are now using the TOKENINFO_URL environment variable which should be supported by Friboo.

See also zalando-stups/taupage#177

When an exception is thrown during system start, it does not get cleaned up.

This concerns only reloaded workflow.

For example, if I mess something up with the database and run (reset), it throws an exception during DB component startup.
After that, I fix the issue and run (reset) again. Now it refuses to start, because the HTTP port is already taken, previous instance of Http component is still running somewhere. (stop) does not help. The only solution here is to restart the REPL, which takes ~20s.

Would be nice to have roll-back functionality.

Serialize SQL Timestamp to Swagger date-time format (ISO)

https://github.com/zalando-stups/friboo/blob/master/src/org/zalando/stups/friboo/system/db.clj#L89
http://xml2rfc.ietf.org/public/rfc/html/rfc3339.html#anchor14

Upgrade swagger ui

A new Swagger UI has just been released, but Friboo is still some releases behind.

Environment is read incorrectly when loading configuration

When running on Linux, there is an environment variable DBUS_SESSION_BUS_ADDRESS that gets picked up as part of :db namespace. We need to only pick up variables starting with DB_, not just DB in this case.

Dependencies for HTTP component have to be specified twice

First in the component definition:

(def-http-component Http "api.yaml" [db tokens])

Then in the system map:

(system/http-system-map configuration
                        map->Http [:db :tokens]
                        :db (db/make-db (:db configuration))
                        :tokens (oauth2/map->OAUth2TokenRefresher ...))

Maintaining these two places is error-prone and non-beginner-friendly.
This also relates to #48.

Provide more info when no operation found

Currently (as of 1.4.0) it does not report the operation name which is not found. Would be nice to have.

ExceptionInfo no operation found  clojure.core/ex-info (core.clj:4593)

Add zookeeper component

Enables applications to automatically use zookeeper in order to for example do locking or member discovery.

Set up Marginalia

In order to have generated documentation, set up Marginalia and the corresponding Leiningen plugin.

Add flyway DB migrations

Add flyway migrations to the DB component. Has to be enabled explictitly.

Add cron scheduler component

Add a cron scheduler component with which you can scheduler functions, leveraging dependency injection similar to the http component.

Hint: integrate with #2 in order to allow :local or :global locking of jobs

Remove WARN log lines for authentication failures

We should not spam the log with WARNINGS about client's missing permissions. I propose changing these log lines to "INFO" and delete some places (e.g. if no auth was given).

Provide runtime REPL

During runtime, the application should open a nREPL port and provide REPL access to the running app.

Look up teams of service users

Because we want to give service users access to their team's application data, e.g. zalando-stups/kio#36 and zalando-stups/mint-storage#9 and also in Pier One.

Is it possible to get rid of log messages?

When the code is loaded (even for uberjar building), the following messages appear:

INFO  [main] o.e.j.u.log - Logging initialized @14782ms
WARN  [main] c.n.c.s.URLConfigurationSource - No URLs will be polled as dynamic configuration sources.
INFO  [main] c.n.c.s.URLConfigurationSource - To enable URLs as dynamic configuration sources, define System property archaius.configurationSource.additionalUrls or make config.properties available on classpath.
INFO  [main] c.n.c.DynamicPropertyFactory - DynamicPropertyFactory is initialized with configuration sources: com.netflix.config.ConcurrentCompositeConfiguration@2f0765f7

Is it possible to make them appear when the system is started, not on class load?

Add /.health endpoint

Add a standard endpoint /.health that returns 200 by default and can optionally be configured with a function, returning a boolean to indicate health (custom application logic).

Audit logging does not work

Log excerpt from a pierone instance:

WARN [pool-2-thread-1] o.z.s.f.s.http - Could not store audit logs because of ["java.lang.Exception: Don't know how to write JSON of class org.eclipse.jetty.server.HttpInput"].

Move Zalando specific components to a separate library

As we are driving Friboo to be generally useful, we should not include components like MgmtHTTP, Metrics, AuditLog or OAuth2TokenRefresher in the default package and of course not in the default system configuration.
Thanks to the component approach, those can easily be moved to a dedicated Zalando specific library.

Use central auth server (magnificent)

Instead of requesting data from 2-3 (closed-source, undocumented) APIs Friboo should only ask one: zalando-stups/magnificent.

Use new team and user API

Change endpoints accordingly as the current ones are deprecated.