Hi Colin,
> With regard to delayMs, can’t we just restart the
> Connector when the keys are actually changed?
Currently the VaultConfigProvider does not find out when values for keys
have changed. You could do this with a poll model (with a
background thread in the ConfigProvider), but since for each key-value
pair, Vault provides a lease duration stating exactly when a value for a
key will change in the future, this is an alternative model of just passing
the lease duration to the client (in this case the Connector), to allow it
to determine what to do (such as schedule a restart). This may allow one
to avoid the complexity of figuring out a proper poll interval (with lease
durations of varying periods), or worrying about putting too much load on
the secrets manager by polling too often. In other words, by adding this
one additional parameter, a ConfigProvider can provide both push and pull
models to clients, perhaps with an additional configuration parameter to
the ConfigProvider to determine which model (push or poll) to use.
Thanks,
Robert
On Wed, May 16, 2018 at 9:56 PM, Colin McCabe <cmcc...@apache.org> wrote:
> Thanks, Robert. With regard to delayMs, can’t we just restart the
> Connector when the keys are actually changed? Or is the concern that
> this would lengthen the effective key rotation time? Can’t the user
> just configure a slightly shorter key rotation time to counteract
> this concern?
> Regards,
> Colin
>
> On Wed, May 16, 2018, at 19:13, Robert Yokota wrote:
> > Hi Colin,
> >
> > Good questions.
> >
> >
> > > As a clarification about the indirections, what if I have the
> > > connect> configuration key foo set up as ${vault:bar}, and in Vault,
> > have the bar> key set to ${file:baz}?
> > > Does connect get foo as the contents of the baz file? I would
> > > argue that> it should not (and in general, we shouldn't allow
> ConfigProviders to
> > indirect to other
> > > ConfigProviders) but I don't think it's spelled out right now.
> >
> > I've added a clarification to the KIP that further indirections are
> > not> performed even if the values returned from ConfigProviders have the
> > variable syntax.
> >
> >
> > > What's the behavior when a config key is not found in Vault
> > > (or other> ConfigProvider)? Does the variable get replaced with the
> empty
> > string, or> with the literal ${vault:whatever} string?
> >
> > It would remain unresolved and still be of the form
> > ${provider:key}. I've> added a clarification to the KIP.
> >
> >
> > > Do we really need "${provider:[path:]key}", or can it just be
> > ${provider:key}?
> >
> > The path is a separate parameter in the APIs, so I think it's
> > important to> explicitly delineate it in the variable syntax. For
> example, I
> > currently> have a working VaultConfigProvider prototype and the syntax
> for a
> > Vault key> reference looks like
> >
> > db_password=${vault:secret/staging:mysql_password}
> >
> > I think it's important to standardize how to separate the path
> > from the key> rather than leave it to each ConfigProvider to determine a
> possibly
> > different way. This will also make it easier to move secrets from one>
> ConfigProvider to another should one choose to do so.
> >
> >
> > > Do we really need delayMs?
> >
> > One of the goals of this KIP is to allow for secrets rotation without>
> having to modify existing connectors. In the case of the
> > VaultConfigProvider, it knows the lease durations and will be able to>
> schedule a restart of the Connector using an API in the Herder. The
> > delayMs will simply be passed to the Herder.restartConnector(long
> > delayMs,> String connName, Callback cb) method here:
> >
> > https://github.com/rayokota/kafka/blob/secrets-in-connect-
> configs/connect/runtime/src/main/java/org/apache/kafka/
> connect/runtime/Herder.java#L170>
> >
> > Best,
> > Robert
> >
> >
> >
> > On Wed, May 16, 2018 at 6:16 PM, Colin McCabe
> > <cmcc...@apache.org> wrote:>
> > > Thanks, Robert. Looks good overall.
> > >
> > > As a clarification about the indirections, what if I have the
> > > connect> > configuration key foo set up as ${vault:bar}, and in Vault,
> have
> > > the bar> > key set to ${file:baz}? Does connect get foo as the
> contents of
> > > the baz> > file? I would argue that it should not (and in general, we
> > > shouldn't allow> > ConfigProviders to indirect to other
> ConfigProviders) but I
> > > don't think> > it's spelled out right now.
> > >
> > > What's the behavior when a config key is not found in Vault
> > > (or other> > ConfigProvider)? Does the variable get replaced with the
> empty
> > > string, or> > with the literal ${vault:whatever} string?
> > >
> > > Do we really need "${provider:[path:]key}", or can it just be
> > > ${provider:key}? It seems like the path can be rolled up into the
> > > key. So> > if you want to put your connect keys under
> my.connect.path, you
> > > ask for> > ${vault:my.connect.path.jdbc.config}, etc.
> > >
> > > > // A delayMs of 0 indicates an immediate change; a positive
> > > > delayMs> > indicates
> > > > // that a future change is anticipated (such as a lease
> > > > duration)> > > void onChange(String path, Map<String, String>
> values, int
> > > > delayMs);> >
> > > Do we really need delayMs? It seems like if you get a callback with>
> > delayMs set, you don't know what the new values will be, only
> > > that an> > update is coming, but not yet here.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Wed, May 16, 2018, at 17:05, Robert Yokota wrote:
> > > > Hello everyone,
> > > >
> > > > After a good round of discussions with excellent feedback and no
> > > > major> > > objections, I would like to start a vote on KIP-297 to
> externalize> > secrets
> > > > from Kafka Connect configurations. My thanks in advance!
> > > >
> > > > KIP: <
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 297%3A+Externalizing+Secrets+for+Connect+Configurations
> > > > >
> > > >
> > > > JIRA: <https://issues.apache.org/jira/browse/KAFKA-6886>
> > > >
> > > > Discussion thread: <
> > > > https://www.mail-archive.com/dev@kafka.apache.org/msg87638.html>
> > > >
> > > > Best,
> > > > Robert
> > >
>
>
