[bartek]

On EKS, pods get AWS access through IAM roles for service accounts (IRSA). The pod is given a web identity token, with AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE set in its environment. The S3 plugin only supports instance, profile and key_secret, so there's no way to use the pod's role. Using instance picks up the node's role instead.

This adds an identity credentialsProvider to the fetcher, emitter and pipes iterator, backed by the SDK's WebIdentityTokenFileCredentialsProvider.

Additionally:

• Added the sts module. The web identity provider needs it to exchange the token for credentials, but doesn't look for it until the first request. Without it, the fetcher builds fine and then fails on its first fetch.
• Moved the fetcher's provider selection into buildCredentialsProvider, like the emitter has, so it can be tested without AWS.

[tballison]

Thank you for opening this.

Let me know what you think of my robot's feedback:

Worth raising:
  - Option name: identity is vague. AWS calls this "web identity" (AssumeRoleWithWebIdentity, AWS_WEB_IDENTITY_TOKEN_FILE), and the existing values use snake_case (key_secret). web_identity fits both, and this name is permanent config surface.
  - EKS Pod Identity still won't work: it's now the AWS-recommended EKS mechanism, and it uses container credentials, not web identity. instance doesn't help, because it only queries the EC2 instance metadata service, so it never sees ECS task roles or Pod Identity. Either add a default option (the SDK's standard chain, which covers both) or say explicitly that Pod Identity is out of scope.
  - Existing docs are wrong:
    - s3.adoc says instance covers "ECS task role, etc.", which is false for the reason above.
    - It also says the fetcher and iterator don't validate the option, but they throw when the component is created.

    The PR edits the lines next to both and keeps both claims.
  - Fourth copy of the provider switch: it extracts the fetcher's switch into a method but leaves the emitter and iterator copies in place. The value list and error message now live in four places. One shared helper would fix that.
  - Tests check mappings, not behavior: they assert which provider class is returned, plus Class.forName for the STS factory.
    That check does catch the important regression (dropping sts), since sts isn't on the classpath any other way. Nothing shows the token is actually exchanged and the result used to sign requests. That's acceptable, but my S3CredentialsTest does exactly that with a local mock STS and S3 if they want it.

  Minor:
  - No CHANGES.txt entry.
  - The same two-line comment is pasted three times.
  - The iterator's error message still has the leftover "…and " + "must be…" string concatenation.

[tballison]

My bot was racing your PR, I can share what it came up with wrt "my" if that'd be of any use.

[bartek]

@tballison Thanks! For context, we had forked apache/tika back when tika-pipes was its own project (via @nddipiazza's effort to support work we were doing there) and added the identity credentials provider to support our deployment needs.

Regarding your machine's feedback, I pushed 3 fixup commits addressing these points:

1. I updated the s3.adoc. I'm not a bug infra guy so I used a machine to help me on this one, using what we've deployed and had success with as context

2. I fixed a small concatenation issue in S3PipesIterator.java

3. On the repeated 2 line comment. None of the other cases have a comment so it's likely best to match that until a documentation sweep is preferred.

On items like:

Fourth copy of the provider switch: it extracts the fetcher's switch into a method but leaves the emitter and iterator copies in place. The value list and error message now live in four places. One shared helper would fix that.

I suppose it's your call? A machine could easily add a helper and refactor, but want to keep the PR scope small.

And:

• No CHANGES.txt entry.

Happy to amend if this is the right PR for it.

And finally:

Option name: identity is vague. AWS calls this "web identity" (AssumeRoleWithWebIdentity, AWS_WEB_IDENTITY_TOKEN_FILE), and the existing values use snake_case (key_secret). web_identity fits both, and this name is permanent config surface.

I feel identity matches the succinct naming terminology already present in these fetchers.

[tballison]

Awesome. I don't see any pushes?

[bartek]

@tballison Apologies; just pushed.

[tballison]

Would it shock your conscience for me to change identity to web_identity? I don't have a phd in s3 options, but I want to make absolutely certain that users know that this is not a different type of identity (like pod identity).

[bartek]

Would it shock your conscience for me to change identity to web_identity? I don't have a phd in s3 options, but I want to make absolutely certain that users know that this is not a different type of identity.

I have no issue with that :) I was trying to keep things close to what exists. The machine makes a good point wrt web_identity

[bartek]

@tballison Appreciate the fast turn around here! We'll validate this end to end under apache/tika as we migrate away from our fork.

[tballison]

Thank you. Please let us know if https://issues.apache.org/jira/browse/TIKA-5001 breaks anything for you.

Also, please, please, please chime in on the work by @krickert and @dpol1 to use more typed objects in our protos (v2!) than all the strings we're using now.

[bartek]

Also, please, please, please chime in on the work my @krickert and @dpol1 to use more typed objects in our protos (v2!) than all the strings we're using now.

Where can I find more information/PRs on this?

[tballison]

#2961

JIRA: https://issues.apache.org/jira/browse/TIKA-4766 and https://issues.apache.org/jira/browse/TIKA-4771

[krickert]

#2961

JIRA: https://issues.apache.org/jira/browse/TIKA-4766 and https://issues.apache.org/jira/browse/TIKA-4771

I would love to talk about it more or create a poc ( Davide already had a demo with storm crawler)

I'd like to demo how an SPI jar can add a grpc parser

I just did the grpc server "fastwarc-grpc" cargo crate in rust. It's an insanely fast warc parser. I wrote that to hope to use in tika.