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

feat : Add support to customize the developer account password (#2359) #4451

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

Conversation

rohanKanojia
Copy link
Contributor

@rohanKanojia rohanKanojia commented Nov 11, 2024

Fixes: Issue #2539

Relates to: Issue #2539

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • I have read the contributing guidelines
  • My code follows the style guidelines of this project
  • I Keep It Small and Simple: The smaller the PR is, the easier it is to review and have it merged
  • I use conventional commits in my commit messages
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I tested my code on specified platforms
    • Linux
    • Windows
    • MacOS

Solution/Idea

  • Generate developer password instead of hardcoded developer string as password
  • Add config option to set developer password for cluster developer-password

Proposed changes

crc would no longer have developer as user password. It would be generated each time cluster is created (just like kubeadmin password). If user wants to override it, they can use developer-password configuration option in CRC config.

Testing

  • After starting CRC cluster user would see a random string as password value instead of `developer
  • If user has specified developer-password configuration option in CRC config, then that value is used in developer password:
    $ crc config set developer-password mypassword
    $ crc start
    [...]
    INFO Adding crc-admin and crc-developer contexts to kubeconfig...
    Started the OpenShift cluster.
    
    The server is accessible via web console at:
      https://console-openshift-console.apps-crc.testing
    
    Log in as user:
      Username: developer
      Password: mypassword

Copy link

openshift-ci bot commented Nov 11, 2024

Hi @rohanKanojia. Thanks for your PR.

I'm waiting for a crc-org 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.

@rohanKanojia rohanKanojia force-pushed the pr/issue2539 branch 4 times, most recently from 6e72835 to 2246b1e Compare November 12, 2024 15:52
@rohanKanojia rohanKanojia marked this pull request as ready for review November 13, 2024 04:08
@openshift-ci openshift-ci bot requested review from cfergeau and gbraad November 13, 2024 04:08
@anjannath
Copy link
Member

/ok-to-test

@rohanKanojia rohanKanojia force-pushed the pr/issue2539 branch 2 times, most recently from 1d67db9 to bf8050f Compare November 18, 2024 08:22
@praveenkumar
Copy link
Member

looks like prow/security is not liking this https://prow.ci.openshift.org/job-history/gs/test-platform-results/pr-logs/directory/pull-ci-crc-org-crc-main-security (recent jobs are green but failed for this)

@rohanKanojia
Copy link
Contributor Author

@praveenkumar : How can I reproduce this failure locally? I'm not able to get much idea about what's wrong by looking at logs:

{"component":"entrypoint","error":"wrapped process failed: exit status 1","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:84","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"2024-11-18T08:45:42Z"}
ERRO[2024-11-18T08:45:43Z] Some steps failed:                           
ERRO[2024-11-18T08:45:43Z] 
  * could not run steps: step security failed: "security" test steps failed: "security" pod "security-openshift-ci-security-snyk-scan" failed: could not watch pod: the pod ci-op-m0wwywcn/security-openshift-ci-security-snyk-scan failed after 44s (failed containers: test): ContainerFailed one or more containers exited

Could it be possible that it's an intermittent failure?

@cfergeau
Copy link
Contributor

The failure is

[Medium] Use of Hardcoded Credentials
   ID: 3373e961-2410-402e-8db9-9c0b595d4e62 
   Path: pkg/crc/config/settings.go, line 32 
   Info: Do not hardcode passwords in code. Found hardcoded saved in DeveloperPassword.

which corresponds to

	DeveloperPassword        = "developer-password"

I think @albfan did some work on snyk's false positives recently (?)

@rohanKanojia
Copy link
Contributor Author

@cfergeau: Thanks for checking. We discussed this in a crc internal Slack chat and decided to add an exception for this particular rule.

@praveenkumar
Copy link
Member

Without this PR the developer user have developer password and it doesn't have a way to change it. There are users who might be using this as scripted way and consuming this well known password. Now with this change either user before hand set the required password or they will get a random password, this looks like a UX change. I think what we should do is have default password set to be developer but provide option to user to change if they want this way the UX is not going to change and existing scripts/automation wouldn't fail. @crc-org/crc-team wdyt?

@rohanKanojia
Copy link
Contributor Author

@praveenkumar : Maybe we can add a configuration named useLegacyDeveloperPassword that would be true by default.

@praveenkumar
Copy link
Member

useLegacyDeveloperPassword

@rohanKanojia I don't think it is good to have multiple config option just for developer user.

…rg#2539)

Add option to set developer password for the user.

```
$ crc config set developer-password mypassword
$ crc start
[...]
INFO Adding crc-admin and crc-developer contexts to kubeconfig...
Started the OpenShift cluster.

The server is accessible via web console at:
  https://console-openshift-console.apps-crc.testing

Log in as user:
  Username: developer
  Password: mypassword
```

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
…value instead

By default developer password would be "developer" that could be
overridden with configuration option developer-password

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Copy link

openshift-ci bot commented Dec 13, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cfergeau for approval. For more information see the 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:

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

Copy link

openshift-ci bot commented Dec 13, 2024

@rohanKanojia: 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/security 5d0cbd2 link false /test security
ci/prow/integration-crc 5d0cbd2 link true /test integration-crc
ci/prow/e2e-crc 5d0cbd2 link true /test e2e-crc

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants