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

Bug 1998432: Support Swift authentication using application credentials #686

Conversation

bverschueren
Copy link
Contributor

This enables image-registry's Swift storage backend to authenticate using application credentials.

Related to https://issues.redhat.com/browse/OSASINFRA-1934

@openshift-ci openshift-ci bot requested review from bparees and dmage May 7, 2021 13:59
@bverschueren
Copy link
Contributor Author

/hold till openshift/docker-distribution#25 lands

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 7, 2021
@bverschueren
Copy link
Contributor Author

cc @dmage

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 9, 2021
@bverschueren bverschueren force-pushed the support-swift-application-credentials branch from 5034db7 to 79e4e34 Compare July 12, 2021 09:30
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2021
@bverschueren
Copy link
Contributor Author

/retest

2 similar comments
@bverschueren
Copy link
Contributor Author

/retest

@bverschueren
Copy link
Contributor Author

/retest

@dmage
Copy link
Contributor

dmage commented Jul 23, 2021

/retitle OSASINFRA-1934: Support Swift authentication using application credentials

@openshift-ci openshift-ci bot changed the title support Swift authentication using application credentials OSASINFRA-1934: Support Swift authentication using application credentials Jul 23, 2021
@dmage
Copy link
Contributor

dmage commented Jul 23, 2021

/approve

@bverschueren do you have QE that can test this feature?

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 23, 2021
@bverschueren
Copy link
Contributor Author

@dmage : @xiuwang will check this out

@xiuwang
Copy link

xiuwang commented Aug 9, 2021

@bverschueren Hi , I have installed cluster on openstack with build from this pr. But there are small issues.
The default secret has installed new auth

 $oc get secret image-registry-private-configuration   -o yaml 
apiVersion: v1
data:
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALID: IiI=
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALNAME: IiI=
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALSECRET: IiI=
  REGISTRY_STORAGE_SWIFT_PASSWORD: ****
  REGISTRY_STORAGE_SWIFT_USERNAME: ****

When use applicationcreditial, the registry pod could be running with new secret, but there is error in registry operator.

$ openstack application credential show wxj 
+--------------+------------------------------------------------------------------+
| Field        | Value                                                            |
+--------------+------------------------------------------------------------------+
| description  | None                                                             |
| expires_at   | None                                                             |
| id           | 7964d5af93ce477c9c35b1286998c516                                 |
| name         | wxj                                                              |
| project_id   | 542c6ebd48bf40fa857fc245c7572e30                                 |
| roles        | swiftoperator _member_                                           |
| system       | None                                                             |
| unrestricted | False                                                            |
| user_id      | 1a0fe8296cf620a99201d6c1039df5c6f986fc23f89bd1c66a2aaaba366bf01f |
+--------------+------------------------------------------------------------------+

$ oc get  secret image-registry-private-configuration  -o yaml 
apiVersion: v1
data:
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALID: Nzk2NGQ1YWY5M2NlNDc3YzljMzViMTI4Njk5OGM1MTYK
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALNAME: d3hqCg==
  REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIALSECRET: bE5JQTA4cUh1NGxm**********
  REGISTRY_STORAGE_SWIFT_PASSWORD: ****
  REGISTRY_STORAGE_SWIFT_USERNAME: ****

 $oc logs -f cluster-image-registry-operator-fb568565f-n76ns | grep -i swift
I0809 06:52:54.255146       1 controller.go:330] object changed: *v1.Config, Name=cluster (status=true): changed:status.conditions.0.lastTransitionTime={"2021-08-09T06:52:53Z" -> "2021-08-09T06:52:54Z"}, removed:status.conditions.0.message="Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF", changed:status.conditions.0.reason={"Could not connect to registry storage" -> "Swift container Exists"}, changed:status.conditions.0.status={"Unknown" -> "True"}, changed:status.conditions.2.lastTransitionTime={"2021-08-09T06:52:53Z" -> "2021-08-09T06:52:54Z"}, changed:status.conditions.2.message={"Unable to apply resources: unable to sync storage configuration: Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF" -> "The registry is ready"}, changed:status.conditions.2.reason={"Error" -> "Ready"}, changed:status.conditions.2.status={"True" -> "False"}

And REGISTRY_STORAGE_SWIFT_APPLICATIONCREDENTIAL** must get together with REGISTRY_STORAGE_SWIFT_USERNAME and REGISTRY_STORAGE_SWIFT_PASSWORD , orelse will prompt no REGISTRY_STORAGE_SWIFT_USERNAME settting.

@bverschueren
Copy link
Contributor Author

@xiuwang: this has a dependency on openshift/docker-distribution#25. The cluster must use an image for the image-registry that has these changes pulled in. I have the image available at quay.io/bverschueren/image-registry:support-swift-application-credentials (built from https://github.com/bverschueren/docker-distribution/tree/support-swift-application-credentials-openshift). So you'll need to use a release image with an updated image-reference for docker-registry to use the image containing the changes.

@xiuwang
Copy link

xiuwang commented Aug 17, 2021

Then I need replace image-registry image with quay.io/bverschueren/image-registry:support-swift-application-credentials .
But image registry operator will recover image-registry setting. If set registry operator to Unmanaged, the registry operator will stand by.
@bverschueren Could we make openshift/docker-distribution#25 merged firstly?
@dmage Do you have suggestion about replacing the image-registry image?
Thanks all

@bverschueren
Copy link
Contributor Author

bverschueren commented Aug 18, 2021

The cluster needs to be created with the updated image for the image-registry. Otherwise the registry fails to authenticate to swift during initialization (without the changes the registry won't consider application credentials to authenticate) and falls back to using cinder for its storage.

So without openshift/docker-distribution/pull/25 merged, one way to test this is to create a release image and provide an alternative image-reference for the docker-registry (e.g.: oc adm release new --from-release <image> --to-image <release-image> docker-registry=quay.io/bverschueren/image-registry:support-swift-application-credentials cluster-image-registry-operator=quay.io/bverschueren/cluster-image-registry-operator:support-swift-application-credentials) and then provide an override image to the installer using OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE

@xiuwang
Copy link

xiuwang commented Aug 24, 2021

@bverschueren I don't create a release image. My steps:

  1. Create a cluster building from this pr, then Scale CVO to 0.
  2. Extract 0000_50_cluster-image-registry-operator_07-operator.yaml and 0000_50_cluster-image-registry-operator_07-operator-service.yaml file from the release image which was gotten from build openshift/cluster-image-registry-operator#686 using cluster-bot
  3. Replace docker-registry to use quay.io/bverschueren/image-registry:support-swift-application-credentials, replace cluster-image-registry-operatore to use quay.io/bverschueren/cluster-image-registry-operator:support-swift-application-credentials in 0000_50_cluster-image-registry-operator_07-operator.yaml
  4. Then apply the two files which will create image-registry-operator and image-registry service and pods.
$oc apply -f 0000_50_cluster-image-registry-operator_07-operator.yaml
$oc apply -f 0000_50_cluster-image-registry-operator_07-operator-service.yaml

Registry operator still reports

I0824 08:59:05.366994       1 controller.go:330] object changed: *v1.Config, Name=cluster (status=true): changed:status.conditions.0.lastTransitionTime={"2021-08-24T08:59:04Z" -> "2021-08-24T08:59:05Z"}, removed:status.conditions.0.message="Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF", changed:status.conditions.0.reason={"Could not connect to registry storage" -> "Swift container Exists"}, changed:status.conditions.0.status={"Unknown" -> "True"}, changed:status.conditions.2.lastTransitionTime={"2021-08-24T08:59:04Z" -> "2021-08-24T08:59:05Z"}, changed:status.conditions.2.message={"Unable to apply resources: unable to sync storage configuration: Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF" -> "The registry is ready"}, changed:status.conditions.2.reason={"Error" -> "Ready"}, changed:status.conditions.2.status={"True" -> "False"}

@bverschueren
Copy link
Contributor Author

@xiuwang I've used this approach to test:

  1. Create cluster from this PR
  2. Scale down CVO
  3. Update IMAGE environment variable for the cluster-image-registry-operator in 0000_50_cluster-image-registry-operator_07-operator.yaml to use the updated image registry:
    - name: IMAGE
      value: quay.io/bverschueren/image-registry:support-swift-application-credentials
    
  4. Apply the manifest
  5. Verify image-registry [after reconciliation]
    $ oc  get co image-registry
    NAME             VERSION                                                  AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
    image-registry   4.9.0-0.ci.test-2021-08-31-082229-ci-ln-bfqt31k-latest   True        False         False      9m33s   
    
    $ oc get pods -n openshift-image-registry -l docker-registry=default
    NAME                             READY   STATUS    RESTARTS   AGE
    image-registry-6d54b5fd5-4rwmp   1/1     Running   0          51s
    image-registry-6d54b5fd5-rcgjw   1/1     Running   0          51s
    
    $ oc  get configs.imageregistry/cluster -o jsonpath='{.status.storage}'|jq
    {
      "managementState": "Managed",
      "swift": {
        "container": "bverschuocpdev-7nbp5-image-registry-mpubgvylmeouhprqiuxkvwgnce"
      }
    }
    

@xiuwang
Copy link

xiuwang commented Sep 1, 2021

@bverschueren I just found my comment has confused you. I didn't claim the swift storage set successfully.
Yes, I could see swift container is reload successfully, after set application credentials.

My point is that registry operator is still reporting below error even the swift container set successfully. But today I found it's a common issue, but not just for configure application creds. I will update bug #1991826 to trace this issue. And add approve label.

I0824 08:59:05.366994       1 controller.go:330] object changed: *v1.Config, Name=cluster (status=true): changed:status.conditions.0.lastTransitionTime={"2021-08-24T08:59:04Z" -> "2021-08-24T08:59:05Z"}, removed:status.conditions.0.message="Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF", changed:status.conditions.0.reason={"Could not connect to registry storage" -> "Swift container Exists"}, changed:status.conditions.0.status={"Unknown" -> "True"}, changed:status.conditions.2.lastTransitionTime={"2021-08-24T08:59:04Z" -> "2021-08-24T08:59:05Z"}, changed:status.conditions.2.message={"Unable to apply resources: unable to sync storage configuration: Failed to authenticate provider client: Post \"https://rhos-d.infra.prod.upshift.rdu2.redhat.com:13000/v3/auth/tokens\": EOF" -> "The registry is ready"}, changed:status.conditions.2.reason={"Error" -> "Ready"}, changed:status.conditions.2.status={"True" -> "False"}

Sorry for the confused make this pr merging delayed.

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Sep 1, 2021
@bverschueren
Copy link
Contributor Author

/retitle Bug 1991826: Support Swift authentication using application credentials

@openshift-ci openshift-ci bot changed the title OSASINFRA-1934: Support Swift authentication using application credentials Bug 1991826: Support Swift authentication using application credentials Sep 2, 2021
@openshift-ci openshift-ci bot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Sep 2, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 2, 2021

@bverschueren: This pull request references Bugzilla bug 1991826, which is invalid:

  • expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1991826: Support Swift authentication using application credentials

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 bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 2, 2021
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@dmage
Copy link
Contributor

dmage commented Sep 23, 2021

@bverschueren the Jira story is Obsolete, should we reassign this PR to another story or close it?

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@bverschueren
Copy link
Contributor Author

@dmage the Jira is superseded by https://bugzilla.redhat.com/show_bug.cgi?id=1998432 (at least wrt Swift) but I mistakenly linked the wrong BZ here previously.

@pierreprinetti
Copy link
Member

/retitle Bug 1998432: Support Swift authentication using application credentials

@openshift-ci openshift-ci bot changed the title OSASINFRA-1934: Support Swift authentication using application credentials Bug 1998432: Support Swift authentication using application credentials Sep 27, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 27, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 27, 2021

@bverschueren: This pull request references Bugzilla bug 1998432, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1998432: Support Swift authentication using application credentials

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.

@pierreprinetti
Copy link
Member

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 27, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 27, 2021

@pierreprinetti: This pull request references Bugzilla bug 1998432, 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.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @eurijon

In response to this:

/bugzilla refresh

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 requested a review from eurijon September 27, 2021 11:48
@pierreprinetti
Copy link
Member

/retest

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

9 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 00f040c into openshift:master Sep 28, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 28, 2021

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

Bugzilla bug 1998432 has been moved to the MODIFIED state.

In response to this:

Bug 1998432: Support Swift authentication using application credentials

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-low Referenced Bugzilla bug's severity is low 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. qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants