Skip to content

Bug 1945548: Filter secret names for registry pod's sa #83

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

Merged
merged 1 commit into from
May 20, 2021

Conversation

hasbro17
Copy link
Contributor

Description of the change:
During the registry server sync the image pull secrets from the catalogsource's spec.secrets are passed
unfiltered to the serviceaccount for the registry pod.

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: ditto-index
  namespace: default
spec:
  sourceType: grpc
  image: quay.io/haseeb_tariq/index-image:ditto-index-0.1
  secrets:
    - ""

Passing an empty string in the secrets list breaks serverside apply for the registry pod which gets created without the metadata.managedFields when it has an empty element in the imagePullSecrets list:

kind: Pod
apiVersion: v1
metadata:
  generateName: ditto-index-
  ...
  # No managedFields on resource
spec:
  imagePullSecrets:
    - {}
    - name: ditto-index-dockercfg-djmz7
...

This prevents the registry pod from being promoted via the SSA client when
there is an update to the index image:

    couldn't ensure registry server - error ensuring updated catalog source pod:
    : detected imageID change: error during update: failed to create manager for
    existing fields: failed to convert new object (/v1, Kind=Pod) to smd typed:
    .spec.imagePullSecrets: element 0: associative list with keys has an element
    that omits key field "name" (and doesn't have default value)

To fix this, the image pull secrets list is filtered for empty strings
before being set on the serviceaccount.

Motivation for the change:
Fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1945548

Similar to kubernetes-sigs/structured-merge-diff#130

Upstream-commit: ee40fd9b8495ea23fc34daef74c689aad942fd2f
Upstream-repository: operator-lifecycle-manager
Upstream PR: operator-framework/operator-lifecycle-manager#2165

During the registry server sync the image pull secrets from the catalogsource's spec.secrets are passed
unfiltered to the serviceaccount for the registry pod.
Passing an empty string in the secrets list breaks serverside apply for the registry pod
with the following error:

failed to convert new object (/v1, Kind=Pod) to smd typed:
.spec.imagePullSecrets: element 0: associative list with keys has an element
that omits key field "name" (and doesn't have default value)

This prevents the registry pod from being promoted via the SSA client when
there is an update to the index image.

To fix this, the image pull secrets list is filtered for empty strings
before being set on the serviceaccount.

Upstream-commit: ee40fd9b8495ea23fc34daef74c689aad942fd2f
Upstream-repository: operator-lifecycle-manager

Signed-off-by: Haseeb Tariq <[email protected]>
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 19, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2021

@hasbro17: This pull request references Bugzilla bug 1945548, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

In response to this:

Bug 1945548: Filter secret names for registry pod's sa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2021
Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dinhxuanvu, hasbro17

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 [dinhxuanvu,hasbro17]

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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2021
@hasbro17
Copy link
Contributor Author

/retest

1 similar comment
@hasbro17
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit ca1f0b6 into openshift:master May 20, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 20, 2021

@hasbro17: All pull requests linked via external trackers have merged:

Bugzilla bug 1945548 has been moved to the MODIFIED state.

In response to this:

Bug 1945548: Filter secret names for registry pod's sa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants