[
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988653#comment-16988653
]
Ashish Chopra commented on SLING-8869:
--------------------------------------
bq. DistributionTransportSecretProvider is an API and anyone can provide an
implementation for it.
Agreed. But as long as one is using {{RemoteDistributionPackageImporter}} (and
being an impl, the only way to _use_ it would be a via creation of configs for
{{DistributionAgentFactory}} impls and
{{RemoteDistributionPackageImporterFactory}}), I still don't see in which case
a {{DistributionTransportSecretProvider}} can be created/updated _without_ a
corresponding creation of new instance of {{RemoteDistributionPackageImporter}}.
{quote}
bq. Can you please help me understand the benefit of a failing HTTP call as you
propose?
It covers both dynamic and static secret providers without making any
assumption about the providers being dynamic or static.
{quote}
None of the secret-providers can actually 'refresh' the token on a 401 because
no such method exists in the API [0]. Depending on their impls they'd either
have to cache+refresh the credentials on every call, or just keep returning the
stale ones.
My concerns with 401/403 based executor eviction are:
* evicting a perfectly good HTTP Client (that backs the executor instance)
(afaik HTTP components advise HTTP Client reuse [1]) when all we need a header
value update
* retry semantics would need review (currently, a
{{RecoverableDistributionException}} is thrown for a 401/404 - its behaviour in
light of how the retry-attempt-counting and moving to error-queue works [2]
would likely need to be reviewed deeply)
[0]
https://github.com/apache/sling-org-apache-sling-distribution-api/blob/master/src/main/java/org/apache/sling/distribution/transport/DistributionTransportSecretProvider.java#L45
[1]
https://hc.apache.org/httpclient-3.x/performance.html#Reuse_of_HttpClient_instance
[2]
https://github.com/apache/sling-org-apache-sling-distribution-core/blob/af3140fc5641c6c3eeee94b1bbc4dbd4013965e6/src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java#L147-L162
> SimpleHttpDistributionTransport does not refresh the secret for token based
> implementations.
> --------------------------------------------------------------------------------------------
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
> Issue Type: Bug
> Components: Content Distribution
> Reporter: Mohit Arora
> Assignee: Timothee Maret
> Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}}
> map, it is not expected that the secret associated with the executor could be
> expired. This can happen in case of access token based implementations where
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the
> executor is [cached in the
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
> the secrets are not refreshed. It works fine for credentials based secret
> provider but not for access token based.
> cc - [~marett]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
