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

Create default passwords when dev mode is set. #441 #442

Merged
merged 43 commits into from
Jan 7, 2025

Conversation

cmoulliard
Copy link
Contributor

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…a new password. cnoe-io#441

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard
Copy link
Contributor Author

Question: Is it the best place to add the code able to generate a new password (= developer) when devMode is enabled -

if result, err := argocd.Install(ctx, resource, r.Client, r.Scheme, r.Config); err != nil {
? @nabuskey

@nabuskey
Copy link
Collaborator

nabuskey commented Nov 8, 2024

Yeah after install() comes back with no errors.

… - developer. Create a kubeClient part of the k8s util package. cnoe-io#441

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard

This comment was marked as resolved.

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard cmoulliard marked this pull request as ready for review November 8, 2024 17:21
@cmoulliard cmoulliard requested a review from nabuskey November 8, 2024 17:32
@cmoulliard
Copy link
Contributor Author

If what I did cannot work, we have 2 options:

  • Create an Argocd PR to propose to use password instead of generating one usng a new parameter --dev or --insecure-password or --dev-password`
    OR
  • Add to our project the 2 Kube secrets (argocd-secret and argocd-initial-admin-secret) with content populated and where we set the property adminAccount.Enabled to false

@nabuskey
Copy link
Collaborator

nabuskey commented Nov 9, 2024

I am still not sure if the flag name is what we want to go with.

Is proving static password the only thing needed for dev mode? Does it describe what it does? If we add more config changes for dev mode in the future, do we want to provide ways to control each change?

If the names I proposed in the issue are too long, can we get away with a short flag?

I do like the idea of having a static default passwords for sure.

@cmoulliard
Copy link
Contributor Author

Is proving static password the only thing needed for dev mode? Does it describe what it does? If we add more config changes for dev mode in the future, do we want to provide ways to control each change?

If we continue to develop idpbuilder, having a devMode help a lot as we do for quarkus devMode able to launch testcontainers: backstage, DB, argocd, gitea, etc and where developers can access many information using dev UI - https://quarkus.io/guides/dev-mode-differences#dev-mode-features :-)

@cmoulliard
Copy link
Contributor Author

If the names I proposed in the issue are too long, can we get away with a short flag?

Let's review that later when we have a PR which is working as --dev is short, enough to test the PR

@cmoulliard
Copy link
Contributor Author

I do like the idea of having a static default passwords for sure.

I will make a try to see if that works

@cmoulliard

This comment was marked as resolved.

@cmoulliard
Copy link
Contributor Author

cmoulliard commented Nov 12, 2024

Apparently, we should be (to be investigated of course) to set the accounts directly within the argocd secret as discussed here: argoproj/argo-cd#4096 (comment)

I was able to play with a user developer where I installed or customized some resources.

RBAC

apiVersion: v1
kind: ConfigMap
metadata:
  labels:
    app.kubernetes.io/name: argocd-rbac-cm
    app.kubernetes.io/part-of: argocd
  name: argocd-rbac-cm
  namespace: argocd
data:
  policy.csv: |
    p, role:developer, applications, *, *, allow
    g, developer, role:developer

ConfigMap

apiVersion: v1
kind: ConfigMap
metadata:
  labels:
    app.kubernetes.io/name: argocd-cm
    app.kubernetes.io/part-of: argocd
  name: argocd-cm
  namespace: argocd
data:
  accounts.developer: apiKey, login
  application.resourceTrackingMethod: annotation
  resource.exclusions: |
    - kinds:
        - ProviderConfigUsage
      apiGroups:
        - "*"
  timeout.reconciliation: 60s

As the following secret is created on the flight by argocd when admin.enabled and generate an admin password using bcrypt, we should find a way to patch it with the developer password

Argocd Secret

apiVersion: v1
kind: Secret
metadata:
  labels:
    app.kubernetes.io/name: argocd-secret
    app.kubernetes.io/part-of: argocd
  name: argocd-secret
  namespace: argocd
type: Opaque
data:
  accounts.developer.password: JDJhJDEwJEYwakhFSmJMYTEvREQzWUZsOWtxN2U0TUZXUXdDclhZWXZNTXhYVXEwdGhVWWUuLi9WWUZL
  accounts.developer.passwordMtime: MjAyNC0xMS0xMlQwODo1NDoyNVo=
...

…veloper's password. cnoe-io#441

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard
Copy link
Contributor Author

cmoulliard commented Nov 12, 2024

The PR supports now to log on to the UI of Argocd using (developer/developer) with RBAC Applications - Admin or gitea (developer/developer)

Can you please review it ?
@nabuskey

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…oper

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard
Copy link
Contributor Author

Apparently we cannot use for gitea developer/developer as we got such an error reported when we create a cluster

Nov 12 11:52:39 ERROR Reconciler error controller=gitrepository controllerGroup=idpbuilder.cnoe.io controllerKind=GitRepository name=argocd namespace=idpbuilder-dabou namespace=idpbuilder-dabou name=argocd reconcileID=1f80e800-276a-4b74-a868-b54ff4a26de5 err=creating repository: failed to create git repository: The repository with the same name already exists. 
Nov 12 11:52:39 ERROR Reconciler error controller=gitrepository controllerGroup=idpbuilder.cnoe.io controllerKind=GitRepository name=nginx namespace=idpbuilder-dabou namespace=idpbuilder-dabou name=nginx reconcileID=798effc7-600a-4035-8e39-eb8bcdcb79b3 err=creating repository: failed to create git repository: The repository with the same name already exists. 
Nov 12 11:52:39 ERROR Reconciler error controller=gitrepository controllerGroup=idpbuilder.cnoe.io controllerKind=GitRepository name=gitea namespace=idpbuilder-dabou namespace=idpbuilder-dabou name=gitea reconcileID=bbb8fbdd-2803-4904-b0c5-c680e2a9c0b9 err=creating repository: failed to create git repository: The repository with the same name already exists. 

I will then revert to the user giteaAdmin in the meantime

… fails

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…for argocd

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

If you are having problems setting passwords on install, we can change them through APIs after install.

namespace: argocd
data:
policy.csv: |
p, role:developer, applications, *, *, allow
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of having a separate account that has a very similar permissions as admin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an account for the developers and it only allows to handle applications

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you tell me if this is inline with what you are thinking?

  1. Use a random password for the developer and admin account if the dev password flag is unset.
  2. Use a static, known password for the developer and admin account if the dev password flag is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Option 2 => Use a known password for the developer and admin account if the dev password flag is set

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok that sounds good to me. Looks like Gitea static password isn't working for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok that sounds good to me. Looks like Gitea static password isn't working for some reason?

For gitea when using dev-mode, we should still use as user: giteaAdmin and password = developer

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are adding a user called developer to argocd, we may as well add the developer user in Gitea.

return ctrl.Result{}, fmt.Errorf("Error marshalling patch data: %w", err)
}

kubeClient, err := k8s.GetKubeClient()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reconciler has client already. You can just use that instead. e.g. r.Client.. You can also change passwords through ArgoCD api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can also change passwords through ArgoCD api.

That will require to be done in a 2nd step while here the idea is to have the account developer/developer available when packages are installed too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

r.Client fixed. See: 7d81b27

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also change passwords through ArgoCD api.

That will require to be done in a 2nd step while here the idea is to have the account developer/developer available when packages are installed too

Yeah I am starting to think we should set them through API for both Gitea and ArgoCD in every invocation of idpbuilder. Instead of relying on manifests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should then (maybe) create some job(s) which are applied post installation of the core package like gitea or argocd and able to execute some additional steps based on parameters passed.

Until now, the first action could be to create a new user: developer able to log on using as user/pwd => developer/developer or idpbuilder/developer if the parameter passed is --dev-mode.

A second job could also patch existing resources if by example we pass a different DNS domain = --host

etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

flag renamed from "dev" to "dev-mode". See: a8c3016

Comment on lines 102 to 111
err = kubeClient.Get(ctx, client.ObjectKey{Name: argocdAdminSecretName, Namespace: argocdNamespace}, &s)
if err != nil {
return ctrl.Result{}, fmt.Errorf("getting argocd secret: %w", err)
}

// Patching the argocd-secret with the user's hashed password
err = kubeClient.Patch(ctx, &s, client.RawPatch(types.StrategicMergePatchType, patchBytes))
if err != nil {
return ctrl.Result{}, fmt.Errorf("Error patching the Secret: %w", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use server side apply instead here to simplify this process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I will review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by "server side" ? My code is similar to what we do here with the Gitea -

return r.Client.Patch(ctx, &u, client.Apply, client.ForceOwnership, client.FieldOwner(v1alpha1.FieldManager))
...

Copy link
Collaborator

@nabuskey nabuskey Nov 21, 2024

Choose a reason for hiding this comment

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

The way it's done in Gitea is server side apply. If you look further up, you'll see it's working with unstructured object to avoid having ownership of fields we do not care about. Sometimes we see errors like "object has been updated and version doesn't match". Server side apply avoids that.

This talks about it: https://eng.d2iq.com/blog/conflict-resolution-kubernetes-server-side-apply/

Here's where unstructured object is used:

u := unstructured.Unstructured{}
u.SetName(giteaAdminSecret)
u.SetNamespace(giteaNamespace)
u.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Secret"))

Here's another example:

func ApplyAnnotation(ctx context.Context, kubeClient client.Client, obj client.Object, annotations map[string]string, opts ...client.PatchOption) error {
// MUST be unstructured to avoid managing fields we do not care about.
u := unstructured.Unstructured{}
u.SetAnnotations(annotations)
u.SetName(obj.GetName())
u.SetNamespace(obj.GetNamespace())
u.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
return kubeClient.Patch(ctx, &u, client.Apply, opts...)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. See: 80a785b

Comment on lines 26 to 30
argocdDevModePassword = "developer"
argocdAdminSecretName = "argocd-secret"
argocdInitialAdminSecretName = "argocd-initial-admin-secret"
argocdInitialAdminPasswordKey = "argocd-initial-admin-secret"
argocdNamespace = "argocd"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are already defined in other packages I think. Either export them or move them to globals or APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I will review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed 2 non used constants and others are non declared in another go file or package. See: 78e1e40

@@ -20,6 +20,7 @@ import (
const (
recreateClusterUsage = "Delete cluster first if it already exists."
buildNameUsage = "Name for build (Prefix for kind cluster name, pod names, etc)."
devModeUsage = "When enabled, the platform will run the core packages with developer password."
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. I am just not sold on the name dev mode. idpbuilder is primary used for development and testing purposes already. I think we should use more explicit name with a short flag.
  2. Even if we accept dev mode, we shouldn't limit the flag name to just passwords. dev mode could mean a lot of different things. This should be a meta flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. idpbuilder is primary used for development and testing purposes already. I think we should use more explicit name with a short flag.

If this is the case, then we should install gitea and argocd using a non generated password ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2. Even if we accept dev mode, we shouldn't limit the flag name to just passwords. dev mode could mean a lot of different things. This should be a meta flag.

What about using the the parameter: --devPwd or --devPassword as boolean @nabuskey

Copy link
Collaborator

@nabuskey nabuskey Nov 21, 2024

Choose a reason for hiding this comment

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

I am ok with dev-pwd or dev-password since we use snake case for our flags. We can also have --dev-mode flag that is essentially a convenient flag that enables dev passwords and any other QOL stuff for dev purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally we aligned our paths => I will then use --dev-mode

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I meant to have two flags. (--dev-password OR --dev-pwd ) AND --dev-mode because we could enable other QOL features under --dev-mode other than static passwords.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be fair is the --dev- prefix needed? I would imagine we could just have a --static-creds parameter or --static-credentials to set the password statically rather then dynamically? Unless there's a reason why we feel --dev- prefix is valuable?

The only argument I could maybe gather is that since it is no longer than admin credentials and it is developer credentials that --dev- prefix gives clarity to that but I think a good description here could explain that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--dev-mode is a pretty common parameter used by many developer's tools and CLI which could also be used to support different additional features in the future. On the opposite --static-creds or --static-credentials parameters are probably too restrictive, don't help the users as they have no idea about what static word means

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I meant to have two flags. (--dev-password OR --dev-pwd ) AND --dev-mode because we could enable other QOL features under --dev-mode other than static passwords.

I still think this needs to be the case.

pkg/k8s/util.go Outdated

"github.com/cnoe-io/idpbuilder/pkg/util"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var (
setupLog = ctrl.Log.WithName("k8s")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather not produce log messages in utility functions. Let's wrap the error instead. e.g. fmt.Errorf("Error creating kubernetes client: %w"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with: 8158551

…orf()

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…ion(s)

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…gocd

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…ssword which is only used internally

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

It's fine to have the dev-mode and dev-password flags. It needs to be represented with clear intent internally in code. The flag names are user friendly but it isn't really descriptive of what it does when referring to the option internally.

When we think about the name dev-password or dev-mode, it's not clear what that means internally. Does dev-password mean we are relaxing password rules? e.g. no special char requirements. Does dev-mode mean we disable TLS?
What we want is to set a static password for all core packages. So we should make this intent clear in code.

@@ -28,6 +28,7 @@ var (

type Build struct {
name string
devMode bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still set as devMode

@@ -67,6 +69,7 @@ func init() {
CreateCmd.PersistentFlags().StringVar(&buildName, "build-name", "localdev", buildNameUsage)
CreateCmd.PersistentFlags().MarkDeprecated("build-name", "use --name instead.")
CreateCmd.PersistentFlags().StringVar(&buildName, "name", "localdev", buildNameUsage)
CreateCmd.PersistentFlags().BoolVar(&devMode, "dev-mode", false, devModeUsage)
Copy link
Collaborator

Choose a reason for hiding this comment

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

add another flag here with the name dev-password.

Copy link
Contributor Author

@cmoulliard cmoulliard Dec 13, 2024

Choose a reason for hiding this comment

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

ok but then what will be the purpose of the flag dev-mode if now we have 2 flags: dev-mode and dev-password ? @nabuskey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed devMode from build.go as non used: 1cd1773

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -143,6 +143,10 @@ func (r *RepositoryReconciler) reconcileGitRepo(ctx context.Context, repo *v1alp
return ctrl.Result{}, fmt.Errorf("getting git provider credentials: %w", err)
}

if r.Config.StaticPassword {
creds.password = "developer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use the string developer in different places. We should define it somewhere and reference it from everywhere it's used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 570ca30

@@ -57,3 +66,20 @@ func (r *LocalbuildReconciler) ReconcileArgo(ctx context.Context, req ctrl.Reque
resource.Status.ArgoCD.Available = true
return ctrl.Result{}, nil
}

func (r *LocalbuildReconciler) ArgocdInitialAdminSecretObject() corev1.Secret {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be moved to utils.

return nil, "succeeded"
}

func (r *LocalbuildReconciler) updateArgocdDevPassword(ctx context.Context, adminPassword string) (error, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change the signature to (ctx context.Context, adminPassword string) error. If it tails for some reason, return an error. If there's an error while updating the password, we should not continue.

return nil, "succeeded"
}

func (r *LocalbuildReconciler) updateArgocdDevPassword(ctx context.Context, adminPassword string) (error, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change the name to updateArgocdPassword. The word dev doesn't mean much internally.

return string(sec.Data["password"]), nil
}

func (r *LocalbuildReconciler) updateGiteaDevPassword(ctx context.Context, adminPassword string) (error, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func (r *LocalbuildReconciler) updateGiteaDevPassword(ctx context.Context, adminPassword string) (error, string) {
func (r *LocalbuildReconciler) updateGiteaPassword(ctx context.Context, adminPassword string) error {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: e4831cd

// Gitea admin secret is not yet available ...
return ctrl.Result{RequeueAfter: defaultRequeueTime}, nil
}
logger.Info("Gitea admin secret found ...")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make this a debug message. It's not useful for normal operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: e4831cd

@@ -89,6 +104,48 @@ func (r *LocalbuildReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}
}

if r.Config.StaticPassword {
logger.V(1).Info("Dev mode is enabled")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
logger.V(1).Info("Dev mode is enabled")
logger.V(1).Info("static passwords enabled")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: e4831cd

)

const (
ArgocdDevModePassword = "developer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ArgocdDevModePassword = "developer"
ArgocdStaticPassword = "developer"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…ile of the util package

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
…e of the functions using it. Convert some messages to log debug messages

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard cmoulliard requested a review from nabuskey January 2, 2025 13:16
@nabuskey
Copy link
Collaborator

nabuskey commented Jan 3, 2025

/e2e

@nabuskey
Copy link
Collaborator

nabuskey commented Jan 3, 2025

E2E is failing but it's unrelated to changes. We need to make issues for:

  • Test coverage for changes in this PR. We need to make sure this flow (use of static passwords) is not disturbed going forward.
  • Document this flag and its purposes.

Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

LGTM

@cmoulliard
Copy link
Contributor Author

cmoulliard commented Jan 4, 2025

  • Test coverage for changes in this PR. We need to make sure this flow (use of static passwords) is not disturbed going forward.

  • Document this flag and its purposes.

I agree. I will create 2 separate issues to improve the testing coverage and documentation
See:

Signed-off-by: cmoulliard <cmoulliard@redhat.com>
@cmoulliard
Copy link
Contributor Author

/e2e

@cmoulliard
Copy link
Contributor Author

e2e succeeded. I propose to merge it then

@cmoulliard cmoulliard merged commit 6c0467e into cnoe-io:main Jan 7, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants