Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-15365: *: use a filtered LIST + WATCH on Secrets for AWS STS #545

Merged

Commits on Jun 29, 2023

  1. *: use a filtered LIST + WATCH on Secrets for AWS STS

    The status quo for this controller is to LIST + WATCH all Secrets on the
    cluster. This consumes more resources than necessary on clusters where
    users put other data in Secrets themselves, as we hold that data in our
    cache and never do anything with it. The reconcilers mainly need to
    react to changes in Secrets created for CredentialRequests, which they
    control and can label, allowing us to filter the LIST + WATCH down and
    hold the minimal set of data in memory. However, two caveats:
    
    - in passthrough and mint mode, admin credentials are also provided by
      the user through Secrets, and we need to watch those, but we can't
      label them
    - on existing clusters, secrets exist from previous work these
      reconcilers did that will not have Secrets labelled
    
    We could solve the second issue with an interim release of this
    controller that labels all previous Secrets, but does not restrict the
    watch stream.
    
    Due to the way that controller-runtime closes over the client/cache
    concepts, it's difficult to solve the first issue, though, since
    we'd need two sets of clients and caches, both for Secrets, and
    ensure that we use one for client access to Secrets we're creating
    or mutating and the other when we're interacting with admin
    credentials. Not impossible to do, but tricky to implement and
    complex.
    
    Until we undertake that effort, we apply a simplification to the
    space: only when AWS STS mode is enabled, we will try to filter the
    LIST + WATCH. This mode is brand new, so we can be reasonably sure
    that there are no previous secrets on the cluster, and, we make the
    filtering best-effort in order to check if that assumption held.
    Second, AWS STS mode only runs in clusters without admin credentials,
    so if we apply the filter, we should not see failures downstream
    from clients that hope to see those objects but can't.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jun 29, 2023
    Configuration menu
    Copy the full SHA
    a58a09c View commit details
    Browse the repository at this point in the history
  2. *: label existing secrets

    In order to reduce the cache footprint we have, we need to label the
    secrets we interact with so we can use a label selector on the LIST +
    WATCH requests we make. The previous commit labels newly-created
    objects, this commit adds a controller that will react to the set of
    secrets on the cluster today and puts labels where they are needed,
    so we can bring clusters without correct labels into conformance.
    
    A future release of the CCO will remove this controller and
    restrict the cache using our new label selector in all cases.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jun 29, 2023
    Configuration menu
    Copy the full SHA
    e84a01c View commit details
    Browse the repository at this point in the history
  3. pkg/operator: expose the status of labelling

    When this controller is somewhere in the middle of labelling the secrets
    it needs to, we will publish a progressing status.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jun 29, 2023
    Configuration menu
    Copy the full SHA
    1dd9815 View commit details
    Browse the repository at this point in the history

Commits on Jun 30, 2023

  1. *: always upsert Secrets

    When we use a LIST + WATCH that is filtered, we will get DELETE events
    when someone removes a label from a Secret that we're watching.
    Similarly, if the controller is down when this happens, the next re-LIST
    will not have the object in the returned set, thereby making our
    controller think that no such secret exists.
    
    We need to amend our logic here, as simply issuing a CREATE call may
    either succeed (if the object does not yet exist) or fail with
    AlreadyExist (if the object is just missing our label).
    
    Thankfully, all of the controllers already implemented this algorithm,
    albeit a little verbosely. This commit makes all of them use the
    upstream controller utilities to achieve the same result.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jun 30, 2023
    Configuration menu
    Copy the full SHA
    1083f6b View commit details
    Browse the repository at this point in the history
  2. pkg/operator: be more explicit in startup check

    We keep the TechPreview feature flag as a precondition for this code
    functioning, but add another logical check for the other precondition we
    have on a filtered watch.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jun 30, 2023
    Configuration menu
    Copy the full SHA
    8d56106 View commit details
    Browse the repository at this point in the history

Commits on Jul 5, 2023

  1. *: use a separate client for root credentials

    If we're using a filtered LIST + WATCH for Secrets to do our work on
    CredentialRequests, we also need to open a second LIST + WATCH for the
    root credential, as this won't have the labels we add to our own
    secrets.
    
    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jul 5, 2023
    Configuration menu
    Copy the full SHA
    358a11f View commit details
    Browse the repository at this point in the history
  2. manifests: add more static related objects

    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jul 5, 2023
    Configuration menu
    Copy the full SHA
    ae2828f View commit details
    Browse the repository at this point in the history
  3. pkg/operator: use cloud cred manager for annotation controller

    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jul 5, 2023
    Configuration menu
    Copy the full SHA
    caf857f View commit details
    Browse the repository at this point in the history

Commits on Jul 10, 2023

  1. pkg/operator: add unit tests for label reconciler

    Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
    stevekuznetsov committed Jul 10, 2023
    Configuration menu
    Copy the full SHA
    c0b4a13 View commit details
    Browse the repository at this point in the history