Proposal: Can we accept vault address in ConfigBuilder?

[jigar23]

ConfigBuilder currently takes host, port, and Tls as separate entries

Vault::Config config = Vault::ConfigBuilder()
            .withHost(Vault::Host{"vault.vault"})
            .withPort(Vault::Port{"8200"})
            .withTlsEnabled(true)

Can we also take in complete address as a parameter so we don't have to split all 3?

Vault::Config config = Vault::ConfigBuilder()
            .withAddress(Vault::Address{"https://vault.vault:8200"})

[abedra]

I'm happy to entertain pull requests for this. The reason I chose not to include this in the initial implementation is that the combination of protocol, host, and port, along with the options around TLS require that you ultimately parse a full URL completely into its constituents so that they can be properly dissected in the underlying client. If you do chose to submit a PR around this, It will need to consider this parser logic and produce a client with the underlying constituents. I definitely understand the motivation behind this and why it would be convenient.

Ultimately, the issue manifests at https://github.com/abedra/libvault/blob/master/src/domain/VaultClient.cpp#L50 when the base URL is derived from the underlying constituents.

[jigar23]

Yeah we just need a parser inside Vault client. I just built a simple parser to get around this because I receive an address and I need to break that in host and port. Please let me know if you can incorporate this inside of the client or I can send a PR

struct UrlParser {
    std::string protocol_;
    std::string host_;
    std::string port_;
    std::string path_;
};

UrlParser parse_url(const std::string &url) {
    UrlParser url_parser;
    if (url.empty()) {
        return url_parser;
    }
    // protocol
    std::size_t start = url.find("://", 0);
    if (start != std::string::npos) {
        // contains protocol
        url_parser.protocol_ = url.substr(0, start);
        start += 3; //"://";
    } else {
        start = 0;
    }
    // host
    bool contains_port = false, contains_path = false;
    std::size_t host_end = url.find(':', start + 1);
    if (host_end != std::string::npos) {
        contains_port = true;
        url_parser.host_ = url.substr(start, host_end - start);
        host_end += 1;
    } else {
        // doesn't contain port
        host_end = url.find('/', start + 1);
        if (host_end != std::string::npos) {
            contains_path = true;
            url_parser.host_ = url.substr(start, host_end - start);
            host_end += 1;
        } else {
            url_parser.host_ = url.substr(start);
        }
    }
    // Port
    std::size_t port_end = 0;
    if (contains_port) {
        port_end = url.find('/', host_end + 1);
        if (port_end != std::string::npos) {
            contains_path = true;
            url_parser.port_ = url.substr(host_end, port_end - host_end);
            port_end += 1;
        } else {
            url_parser.port_ = url.substr(host_end);
        }
    }
    // Path
    if (contains_path) {
        std::size_t path_start = contains_port ? port_end : host_end;
        url_parser.path_ = url.substr(path_start);
    }

    return url_parser;
}

[abedra]

There are two ways to think about it. One is that this client could maintain a parser that parses into either a structure with the constituents, or, given a constructed vault configuration, use the with methods to create the correct version of the client after parsing. The other, would be to have that externally.

I'm open to the idea of libvault having this, but I ask that you submit a PR, complete with unit tests.

[abedra]

Hi @jigar23, are you still interested in making this contribution?