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

📖 Document how to configure global pull secrets #1410

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

anik120
Copy link
Contributor

@anik120 anik120 commented Oct 25, 2024

Description

Closes #1409

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@anik120 anik120 requested a review from a team as a code owner October 25, 2024 12:34
Copy link

netlify bot commented Oct 25, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 405aa50
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/6721165d9be5b600086dce1f
😎 Deploy Preview https://deploy-preview-1410--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.57%. Comparing base (5713012) to head (405aa50).
Report is 47 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1410   +/-   ##
=======================================
  Coverage   73.57%   73.57%           
=======================================
  Files          42       42           
  Lines        3194     3194           
=======================================
  Hits         2350     2350           
  Misses        659      659           
  Partials      185      185           
Flag Coverage Δ
e2e 55.13% <ø> (ø)
unit 53.35% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@anik120 anik120 force-pushed the private-registry-docs branch from ee89046 to f90b98e Compare October 25, 2024 13:03
Copy link
Member

@m1kola m1kola left a comment

Choose a reason for hiding this comment

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

The doc looks good to me.

Not merging it just yet because, if I recall correctly, there was a discussion about this feature and we were not sure if we ready to document it or not.

@anik120 please correct me if I'm wrong.

@anik120
Copy link
Contributor Author

anik120 commented Oct 28, 2024

@m1kola that is correct.

Please hold until we reach some kind of a resolution in this thread https://kubernetes.slack.com/archives/C0181L6JYQ2/p1729859882375089

/hold

@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 Oct 28, 2024
@anik120 anik120 force-pushed the private-registry-docs branch 9 times, most recently from 06398e0 to 602752a Compare October 29, 2024 16:54
@anik120 anik120 force-pushed the private-registry-docs branch from 602752a to 405aa50 Compare October 29, 2024 17:07
- alpha
---

# Configure global pull secrets for allowing components to pull private images
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Configure global pull secrets for allowing components to pull private images
# Configure global pull secrets for allowing components to pull from private registries

I think it would be more clearer to use the term "private registries" instead of "private images," as the authentication is specifically for accessing the registry where images are hosted, right?


# Configure global pull secrets for allowing components to pull private images

**Note: The UX for how auth info for using private images is provided is an active work in progress.**
Copy link
Contributor

@camilamacedo86 camilamacedo86 Nov 5, 2024

Choose a reason for hiding this comment

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

Suggested change
**Note: The UX for how auth info for using private images is provided is an active work in progress.**
**Note: The UX for providing authentication details for private registry images is an active work in progress.**

same ^


**Note: The UX for how auth info for using private images is provided is an active work in progress.**

To configure `catalogd` and `operator-controller` to use authentication information for pulling private images (catalog/bundle images etc), the components can be informed about a kubernetes `Secret` object that contains the relevant auth information. The `Secret` must be of type `kubernetes.io/dockerconfigjson`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To configure `catalogd` and `operator-controller` to use authentication information for pulling private images (catalog/bundle images etc), the components can be informed about a kubernetes `Secret` object that contains the relevant auth information. The `Secret` must be of type `kubernetes.io/dockerconfigjson`.
To allow [catalogd](https://github.com/operator-framework/catalogd) and [operator-controller](https://github.com/operator-framework/operator-controller) to authenticate and pull images from a private registry (for catalog/bundle images, etc.), you need to provide these components with a Kubernetes `Secret` object containing the necessary credentials. The `Secret` must be of type [`kubernetes.io/dockerconfigjson`](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials).
  • The correct term would be "private registry," right, not "images"?
  • Additionally, wdyt about adding links to both project repositories for catalogd and operator-controller?
  • wdyt about including a reference to the Kubernetes documentation on kubernetes.io/dockerconfigjson secrets for further guidance?


To configure `catalogd` and `operator-controller` to use authentication information for pulling private images (catalog/bundle images etc), the components can be informed about a kubernetes `Secret` object that contains the relevant auth information. The `Secret` must be of type `kubernetes.io/dockerconfigjson`.

Once the `Secret` is created, `catalogd` and `operator-controller` needs to be redeployed with an additional field, `--global-pull-secret=<secret-namespace>/<secret-name>` passed to the respective binaries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Once the `Secret` is created, `catalogd` and `operator-controller` needs to be redeployed with an additional field, `--global-pull-secret=<secret-namespace>/<secret-name>` passed to the respective binaries.
Once the `Secret` is created, `catalogd` and `operator-controller` needs to be redeployed with an additional flag, --global-pull-secret=<secret-namespace>/<secret-name>, to pass the registry credentials to the respective binaries.


Once the `Secret` is created, `catalogd` and `operator-controller` needs to be redeployed with an additional field, `--global-pull-secret=<secret-namespace>/<secret-name>` passed to the respective binaries.

For eg, create a `Secret` using locally available `config.json`:
Copy link
Contributor

@camilamacedo86 camilamacedo86 Nov 5, 2024

Choose a reason for hiding this comment

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

Suggested change
For eg, create a `Secret` using locally available `config.json`:
For example, to create a `Secret` using locally available `config.json` for the private registry:

$ kubectl create secret docker-registry test-secret \
--from-file=.dockerconfigjson=$HOME/.docker/config.json \
--namespace olmv1-system
secret/test-secret created
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include the output of the command? Adding it might make it harder for users to copy, paste, and use the command directly, right?

type: kubernetes.io/dockerconfigjson
```

Modify the `config/base/manager/manager.yaml` file for `catalogd` and `operator-controller` to include the new field in the binary args:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Modify the `config/base/manager/manager.yaml` file for `catalogd` and `operator-controller` to include the new field in the binary args:
Modify the `config/base/manager/manager.yaml` file for `catalogd` and `operator-controller` to include the new flag in the binary args:

- --global-pull-secret=olmv1-system/test-secret
```

With the above configuration, creating a `ClusterCatalog` or a `ClusterExention` whose content is packaged in a private container image hosted in an image registry, will become possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
With the above configuration, creating a `ClusterCatalog` or a `ClusterExention` whose content is packaged in a private container image hosted in an image registry, will become possible.
With the above configuration, creating a `ClusterCatalog` or a `ClusterExention` whose content is packaged in a container image hosted in an private registry, will become possible.

is the registry which is private right?

@@ -14,6 +14,8 @@ After you add a catalog to your cluster, you can install an extension by creatin
* The name, and optionally version, or channel, of the [supported extension](../project/olmv1_limitations.md) to be installed
* An existing namespace in which to install the extension

**Note** To install ClusterExentions that are shipped as private container images hosted in an image registry, please see [How to conifgure global pull secrets](../howto/configure-global-pull-secrets.md).
Copy link
Contributor

@camilamacedo86 camilamacedo86 Nov 5, 2024

Choose a reason for hiding this comment

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

Suggested change
**Note** To install ClusterExentions that are shipped as private container images hosted in an image registry, please see [How to conifgure global pull secrets](../howto/configure-global-pull-secrets.md).
**Note** To install `ClusterExentions` that are shipped as container images hosted in an private registry, please see the HowTo [Configure global pull secrets for allowing components to pull from private registries](../howto/configure-global-pull-secrets.md).

Just to match the title

@@ -36,6 +37,7 @@ nav:
- Uninstall an Extension: tutorials/uninstall-extension.md
- How-To Guides:
- Catalog queries: howto/catalog-queries.md
- Configure Global pull secrets: howto/configure-global-pull-secrets.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Configure Global pull secrets: howto/configure-global-pull-secrets.md
- Configure global pull secrets to pull from private registries: howto/configure-global-pull-secrets.md

maybe a little more informative wdyt?

@m1kola m1kola added the documentation Improvements or additions to documentation label Nov 19, 2024
@m1kola m1kola added the kind/documentation Categorizes issue or PR as related to documentation. label Nov 19, 2024
- ...
- ...
- ...
- --global-pull-secret=olmv1-system/test-secret
Copy link
Contributor

Choose a reason for hiding this comment

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

If we edit the deployment directly, we need to re-install.
Then, if we run make run we will lose what was done above
Also, it does not seems a valid approach for end users

So, I think we need say here to:

kubectl patch deployment operator-controller-controller-manager -n olmv1-system --type=json -p='[
  {
    "op": "add",
    "path": "/spec/template/spec/containers/0/args/-",
    "value": "--global-pull-secret=olmv1-system/test-secret"
  }
]'

@LalatenduMohanty
Copy link
Member

@anik120 Is this PR ready to get merged?

@camilamacedo86
Copy link
Contributor

Hi @anik120

See that I tried the steps here and did not work: https://redhat-internal.slack.com/archives/C06KP34REFJ/p1732973720649019

Seems required to also use the flag --global-pull-secret flag to the catalogd manager deployment as well,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. documentation Improvements or additions to documentation kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Support installing bundles from a private registry
4 participants