support marathon basic auth (Bearer)

[adamdecaf]

Following from #3090 there's no support for Authorization: Basic ... with marathon, which is what non DC/OS installs use.

Part of the auth_token changes mention this: 2aba238

Existing configuration files that use the bearer token will no longer work. More work is required to make this backwards compatible.

Even before the auth_token[_file] changes this wouldn't have worked.

https://github.com/prometheus/prometheus/blob/v2.2.1/discovery/marathon/marathon.go#L300-L305

[adamdecaf]

FYI, I've already got a local change support both auth types. I'll PR that when it's ready.

cc @plaflamme

[brian-brazil]

This was added in #4009.

[plaflamme]

@adamdecaf Can you describe what doesn't work exactly? The changes in #4009 should have added support for all authentication types:

• basic auth (for vanilla Marathon)
• auth token (for DC/OS Marathon)
• bearer token (for future DC/OS Marathon, according to their documentation)

[adamdecaf]

In specific the code here didn't seem to work for me. I need an auth header like Authorization: Basic ....

request.Header.Set("Authorization", "token="+string(rt.authToken))

Is there a pre-release docker image / release with #4009? I can test that out.

[adamdecaf]

What I'm seeing is that the auth plugin we use needs Basic ..., which prometheus doesn't seem to allow.

    public static AuthKey authKeyFromHeaders(HttpRequest request) throws Exception {
        Option<String> header = request.header("Authorization").headOption();
        if (header.isDefined() && header.get().startsWith("Basic ")) {
            String encoded = header.get().replaceFirst("Basic ", "");
            String decoded = new String(Base64.getDecoder().decode(encoded), "UTF-8");
            String[] userPass = decoded.split(":", 2);

            return AuthKey.with(userPass[0], userPass[1]);
        }
        return null;
    }

https://github.com/ContainX/marathon-ldap/blob/master/src/main/java/io/containx/marathon/plugin/auth/util/HTTPHelper.java#L19

[plaflamme]

Ah, ok. That's because you need your config to use a basic_auth block as described in the updated documentation https://github.com/prometheus/prometheus/blob/master/docs/configuration/configuration.md#marathon_sd_config

[adamdecaf]

Oh ok. The commits made it seem like basic/bearer auth were removed. Are they going to be around for a while? We're not going to use DC/OS.

Thanks. I'll try building a docker image from master locally with bearer_token_file.

[plaflamme]

@adamdecaf basic auth was not supported before #4009. Supporting it is what motivated the change.

All authentication methods should be supported in the long run. I suspect the auth_token method will be dropped if / when DC/OS moves to the more standard bearer_token authentication.

[adamdecaf]

Perfect, thanks!

[adamdecaf]

@brian-brazil @plaflamme Would y'all accept a basic_auth_file PR? I want to keep the basic auth params in a kubernetes Secret (mounted into the prometheus container).

[brian-brazil]

I wouldn't accept a file that contains basic auth details as that'd make debugging harder by making the username not available in the UI, if we were to accept it it would have to contain no more than the raw password.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.