Read secrets for onboarding-token validation #2790

mrudraia1 · 2024-09-10T09:10:01Z

This PR is a copy PR of #2715

I faced git account issues, I was not able to recover my account, so created a new PR with new account

This PR reads the secrets instead of reading the secrets from the volume mounts.
whenever the new onboarding secrets are created, it takes more time to read the secrets from the volume mounts,
The user clicks the rotate onboarding keys, the kubernetes still uses the old public, private keys , the new keys are mounted later, So this PR will read the secrets directly from the kubernetes secrets.

openshift-ci · 2024-09-10T09:10:06Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mrudraia1
Once this PR has been reviewed and has the lgtm label, please assign jarrpa for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

openshift-ci · 2024-09-10T09:10:13Z

Hi @mrudraia1. Thanks for your PR.

I'm waiting for a red-hat-storage member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

malayparida2000 · 2024-09-17T04:35:13Z

/ok-to-test

mrudraia1 · 2024-09-17T05:29:27Z

/ok-to-test

leelavg · 2024-09-17T05:34:12Z

/ok-to-test

leelavg

much better, can merge it at the earliest.

leelavg · 2024-09-17T05:36:37Z

services/ux-backend/main.go

 )

 type serverConfig struct {
+	client.Client


where is this being used?

removed, it is no were called

leelavg · 2024-09-17T05:38:36Z

services/ux-backend/main.go

@@ -7,12 +7,17 @@ import (
 	"os"
 	"strconv"

-	"k8s.io/klog/v2"
-
+	v1 "github.com/red-hat-storage/ocs-operator/api/v4/v1"


pls change to ocsv1, we generally use v1 for native apis, even that I'm not seeing lately.

leelavg · 2024-09-17T05:40:33Z

controllers/util/util.go

@@ -5,6 +5,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

+const onboardingValidationPrivateKeySecretName = "onboarding-private-key"


could you pls move this const to the file where it is being used?

leelavg · 2024-09-17T05:42:46Z

controllers/util/onboarding_secrets.go

+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func ReadPrivateKey(cl client.Client) (*rsa.PrivateKey, error) {


why can't this function be in existing provider.go itself?

In Provider.go, their will be import issues for package
"crypto", "crypto/rand", "crypto/rsa". ReadPrivateKey function require "crypto/rsa", "crypto/x509". So for import conflicts, I created an new file.

could you pls expand on this, I don't see the issue.

Ack, deleted the onboarding_secrets.go and function called in provider.go

leelavg · 2024-09-17T05:44:22Z

controllers/util/onboarding_secrets.go

+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func ReadPrivateKey(cl client.Client) (*rsa.PrivateKey, error) {


we aren't reading the key, we are getting it from API and parsing it as well, name GetParsedPrivateKey is more apt.

leelavg · 2024-09-17T05:46:26Z

controllers/util/onboarding_secrets.go

+	klog.Info("Getting the Pem key")
+	ctx := context.Background()
+
+	operatorNamespace, err := GetOperatorNamespace()


get the namespace as param rather than reading it everytime, I see this is being used in two places where you can already have ns.

in reconciler, already has it

in handler, pls initialize ns once and read from it rather than reading from env everytime, ref remove rotation of keys endpoint from ux-backend #2569

rchikatw · 2024-09-17T05:53:43Z

controllers/storagecluster/storageclient.go

+	privateKey, err := util.ReadPrivateKey(r.Client)
+	if err != nil {
+		r.Log.Error(err, "Unable to get privatekey")
+		return reconcile.Result{}, nil


you might want to return an error here.

openshift-ci · 2024-09-17T13:30:48Z

@mrudraia1: This pull request references Bugzilla bug 231102, which is valid.

No validations were run on this bug

In response to this:

Bug 231102: [release-4.17] Read secrets for onboarding-token validation

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-sigs/prow repository.

leelavg · 2024-09-17T14:23:59Z

We add Bug <num>:[branch] to PR summary when we are back porting to that branch but not when targeting main

openshift-ci · 2024-09-24T08:27:44Z

@mrudraia1: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Read secrets for onboarding-token validation

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-sigs/prow repository.

openshift-merge-robot · 2024-09-27T23:29:29Z

PR needs rebase.

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-sigs/prow repository.

While adding the fields the description of the fields were directly lifted off from rook-operator. The values of NearFull, BackfillFull & Full in rook are 0.85, 0.90 & 0.95 respectively. But in OCS Operator we set these values to 0.75, 0.80 & 0.85 respectively with the help of the rook-config-override ConfigMap. So the description of the fields in the API should reflect the actual values that are set in OCS. Signed-off-by: Malay Kumar Parida <[email protected]>

Earlier the cluster utilization alert rules (CephClusterNearFull, CephClusterCriticallyFull, CephClusterReadOnly) and the osd alert rules (CephOSDNearFull, CephOSDCriticallyFull) were hardcoded to use the nearFullRatio 0.75, criticallyFullRatio 0.80, and fullRatio 0.85 values. But these values are now configurable on the storageCluster CR. So the prometheus rules for these alerts will now be updated to use the specified values if provided in the storageCluster CR. This also includes the refactor of the changing the prometheus rule process. The function is now easier to read, maintain & expand. Also add tests for prometheus rule changing process. Signed-off-by: Malay Kumar Parida <[email protected]>

The PR does the following: 1. add role to the onboarding ticket 2. add a new endpoint for peer-onboarding-tokens Signed-off-by: Rewant Soni <[email protected]>

The commit does the following: 1. Create service, deployment, onboarding job for both modes 2. Update the variable from watchnamespace to podnamespace 3. Remove hardcoded name for storagecluster 4. Move client configmap in storageclient Signed-off-by: Rewant Soni <[email protected]>

Signed-off-by: Rewant Soni <[email protected]>

Signed-off-by: Kaustav Majumder <[email protected]>

Signed-off-by: Mrudraia1 <[email protected]>

openshift-ci · 2024-09-30T08:59:30Z

@mrudraia1: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/ci-bundle-ocs-operator-bundle	`75c8e1b`	link	true	`/test ci-bundle-ocs-operator-bundle`
ci/prow/images	`75c8e1b`	link	true	`/test images`
ci/prow/ocs-operator-bundle-e2e-aws	`75c8e1b`	link	true	`/test ocs-operator-bundle-e2e-aws`

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

mrudraia1 · 2024-09-30T09:53:12Z

reopened new PR 2827. closing this PR due to some conflicting files and commits

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 10, 2024

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 17, 2024

mrudraia1 force-pushed the onboarding1 branch from b8c63f3 to fa91ba7 Compare September 17, 2024 05:28

leelavg reviewed Sep 17, 2024

View reviewed changes

rchikatw reviewed Sep 17, 2024

View reviewed changes

mrudraia1 force-pushed the onboarding1 branch from fa91ba7 to 9493a36 Compare September 17, 2024 08:56

mrudraia1 changed the title ~~Read secrets for onboarding-token validation.~~ Read secrets for onboarding-token validation Sep 17, 2024

mrudraia1 force-pushed the onboarding1 branch from 9493a36 to 68981e6 Compare September 17, 2024 09:21

mrudraia1 requested review from leelavg and rchikatw September 17, 2024 09:59

mrudraia1 force-pushed the onboarding1 branch from 68981e6 to 05a2905 Compare September 17, 2024 10:58

mrudraia1 changed the title ~~Read secrets for onboarding-token validation~~ Bug 231102: [release-4.17] Read secrets for onboarding-token validation Sep 17, 2024

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 Sep 17, 2024

mrudraia1 changed the title ~~Bug 231102: [release-4.17] Read secrets for onboarding-token validation~~ Bug 231102: Read secrets for onboarding-token validation Sep 18, 2024

mrudraia1 changed the title ~~Bug 231102: Read secrets for onboarding-token validation~~ Read secrets for onboarding-token validation Sep 24, 2024

openshift-ci bot removed 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 Sep 24, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2024

mrudraia1 force-pushed the onboarding1 branch from 05a2905 to 5da770f Compare September 30, 2024 07:54

malayparida2000 and others added 8 commits September 30, 2024 14:00

add new endpoint for onboarding peers

f3f92f0

The PR does the following: 1. add role to the onboarding ticket 2. add a new endpoint for peer-onboarding-tokens Signed-off-by: Rewant Soni <[email protected]>

add generated changes for csv and rbac

0370aba

Signed-off-by: Rewant Soni <[email protected]>

fix unit tests

8245eeb

Signed-off-by: Rewant Soni <[email protected]>

Prevent noobaaccount creation in provider cluster

b60699a

Signed-off-by: Kaustav Majumder <[email protected]>

Read secrets for onboarding-token validation

75c8e1b

Signed-off-by: Mrudraia1 <[email protected]>

mrudraia1 force-pushed the onboarding1 branch from 5da770f to 75c8e1b Compare September 30, 2024 08:31

mrudraia1 closed this Sep 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read secrets for onboarding-token validation #2790

Read secrets for onboarding-token validation #2790

mrudraia1 commented Sep 10, 2024

openshift-ci bot commented Sep 10, 2024

openshift-ci bot commented Sep 10, 2024

malayparida2000 commented Sep 17, 2024

mrudraia1 commented Sep 17, 2024

leelavg commented Sep 17, 2024

leelavg left a comment

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

leelavg Sep 17, 2024

mrudraia1 Sep 17, 2024

rchikatw Sep 17, 2024

mrudraia1 Sep 17, 2024

openshift-ci bot commented Sep 17, 2024

leelavg commented Sep 17, 2024

openshift-ci bot commented Sep 24, 2024

openshift-merge-robot commented Sep 27, 2024

openshift-ci bot commented Sep 30, 2024

mrudraia1 commented Sep 30, 2024

Read secrets for onboarding-token validation #2790

Read secrets for onboarding-token validation #2790

Conversation

mrudraia1 commented Sep 10, 2024

openshift-ci bot commented Sep 10, 2024

openshift-ci bot commented Sep 10, 2024

malayparida2000 commented Sep 17, 2024

mrudraia1 commented Sep 17, 2024

leelavg commented Sep 17, 2024

leelavg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 17, 2024

leelavg commented Sep 17, 2024

openshift-ci bot commented Sep 24, 2024

openshift-merge-robot commented Sep 27, 2024

openshift-ci bot commented Sep 30, 2024

mrudraia1 commented Sep 30, 2024