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

WIP: Fix bootstrapping with default credentials #816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nijave
Copy link

@nijave nijave commented May 17, 2024

Description

  • Set a more complex default password--not sure if there are documentation updates that need to go with this.
  • Consolidate env var generation that's the same between STS/Bootstrap into a convient function.
  • Update the example password in docs to fulfill complexity requirements.
  • Remove unused username parameter. Since I'm adding an additional parameter it seemed fair to remove one that appeared unused to limit how many parameters are being passed around. However, it's unclear if this interface is expected to be stable or is consumed outside the codebase.

Issues Resolved

#759

Check List

  • Commits are signed per the DCO using --signoff
  • Unittest added for the new/changed functionality and all unit tests are successful
  • Customer-visible features documented
  • No linter warnings (make lint)

If CRDs are changed:

  • [N/A] CRD YAMLs updated (make manifests) and also copied into the helm chart
  • [N/A] Changes to CRDs documented

Please refer to the PR guidelines before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@nijave nijave force-pushed the nv-fix-password-bootstrap branch 2 times, most recently from 2ed94dc to ec6c66c Compare May 17, 2024 02:22
return "admin", "admin", nil
// minimum 8 character password and must contain at least one uppercase letter,
// one lowercase letter, one digit, and one special character
return "admin", "0penS3@rch!", nil
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure about this part. admin didn't meet the default security requirements. Maybe this should be randomly generated instead (it'll get stored in the Secret object for admins to retrieve).

If this should be randomly generated, I could use some guidance if there is any existing code to assist or commonly used libraries.

@nijave
Copy link
Author

nijave commented May 17, 2024

With these changes, I'm able to bootstrap a cluster using the following CRD

---
apiVersion: opensearch.opster.io/v1
kind: OpenSearchCluster
metadata:
  name: default-cluster
  namespace: opensearch
spec:
  general:
    # Should also have this plugin @ version https://github.com/Aiven-Open/prometheus-exporter-plugin-for-opensearch/
    version: "2.13.0"
    httpPort: 9200
    vendor: opensearch
    serviceName: opensearch
    monitoring:
     enable: true
    pluginsList: ["repository-s3"]
    setVMMaxMapCount: true
  dashboards:
    version: "2.13.0"
    enable: true
    replicas: 1
    resources:
      requests:
         memory: 256Mi
         cpu: 50m
      limits:
         memory: 1Gi
         cpu: 500m
  confMgmt:
    smartScaler: true
  nodePools:
    - component: masters
      replicas: 3
      diskSize: 4Gi
      nodeSelector:
      resources:
         requests:
            memory: 512Mi
            cpu: 50m
         limits:
            memory: 768Mi
            cpu: 1000m
      roles:
        - master
        - ingest
    - component: nodes
      replicas: 3
      diskSize: 40Gi
      nodeSelector:
      resources:
         requests:
            memory: 1Gi
            cpu: 50m
         limits:
            memory: 2Gi
            cpu: 1000m
      roles:
        - data

- Set a more complex default password--not sure if there are
  documentation updates that need to go with this.
- Consolidate env var generation that's the same between STS/Bootstrap
  into a convient function.
- Update the example password in docs to fulfill complexity
  requirements.

Signed-off-by: Nick Venenga <[email protected]>
@nijave nijave force-pushed the nv-fix-password-bootstrap branch from ec6c66c to d46cbe8 Compare May 17, 2024 03:26
@nijave nijave changed the title Fix bootstrapping with default credentials WIP: Fix bootstrapping with default credentials May 19, 2024
@yiippee
Copy link

yiippee commented Jun 10, 2024

Merging is blocked?
Any update on this? Thanks.

@nijave
Copy link
Author

nijave commented Jun 10, 2024

Merging is blocked? Any update on this? Thanks.

I couldn't repro in the CI environment. Not sure why it failed on my cluster (and apparently some other people's)

GitHub Actions CI looks like a standard cluster

@yiippee
Copy link

yiippee commented Jun 11, 2024

I want to generate a random password when opensearch is started, the operator can do that ? Thanks.

}},
},
{
Name: "OPENSEARCH_INITIAL_ADMIN_PASSWORD",
Copy link

Choose a reason for hiding this comment

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

what does the 'OPENSEARCH_INITIAL_ADMIN_PASSWORD' means? I only see the definition here, but I can't find a place to use this environment variable. Did I miss something? Thanks.

Choose a reason for hiding this comment

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

@yiippee it is env. variable which needs to be set prior running the pod. https://opensearch.org/blog/replacing-default-admin-credentials/

@apenen
Copy link

apenen commented Nov 15, 2024

I need this to set up my first 2.18 cluster. What are the next steps? Can I contribute?

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

Successfully merging this pull request may close these issues.

4 participants