Allow custom service account issuer without public bucket

[hakman]

/cc @olemarkus @johngmyers

[hakman]

/test pull-kops-e2e-k8s-aws-calico

[johngmyers]

As stated in Office Hours, I would like there to be more documentation around this use case.

[johngmyers]

I read over the existing documentation. It states:

The enableAWSOIDCProvider configures AWS to trust the service account issuer to
authenticate service accounts for IAM Roles for Service Accounts (IRSA). In order for this to work,
the service account issuer discovery URL must be publicly readable.

This is true, though it should probably more specifically state that spec.kubeAPIServer.serviceAccountIssuer is what must be publicly readable.

The must-be-publicly-readable requirement could be made conditional on enableAWSOIDCProvider: true since it is AWS IAM that makes that requirement. Technically, we should be applying the requirement of being publicly readable to serviceAccountIssuer instead of discoveryStore, but that's a bit difficult now.

We should document the conditions under which kOps sets object-level ACLs. I'm wondering if we should deprecate the object-level ACLs.

[olemarkus]

We should document the conditions under which kOps sets object-level ACLs. I'm wondering if we should deprecate the object-level ACLs.

There is something off with the logic today, I believe. But the object-level ACL is somewhat critical in the cases where public buckets are used as state store (like e.g our e2e tests). But the ACL should only tighten the restrictions. Right now, it is also trying to loosen the restrictions (public objects in private buckets), which fail.

[johngmyers]

I was referring to the PublicACL field of ManagedFile.

There's other code that sets public-read for the fileRepository on S3. That should probably also be deprecated. The fileRepository doesn't have to be public in the first place.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[hakman]

/remove-lifecycle stale

[johngmyers]

I think what we need is to update the documentation in cluster_spec.md stating the conditions in which it will set object-level public-read ACLs, namely that it is in S3, there is no whole-bucket public ACL, and the kubeAPIServer.serviceAccountIssuer hasn't been overridden to point somewhere else.

[justinsb]

We discussed in office hours, this unblocks some users hitting this problem, so we're going to merge this and hopefully document it more fully over time.

Thanks @hakman

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [justinsb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment