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

Change Helm charts to use secret instead of password. #15

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

ebeaty-cisco
Copy link
Contributor

@ebeaty-cisco ebeaty-cisco commented Sep 16, 2024

password XR config is being deprecated in favour of secret. Update the Helm charts to use secret instead of password.

UT written for all changes

Tested manually on vRouter and Control Plane images (XR version 7.7).

@ebeaty-cisco ebeaty-cisco marked this pull request as ready for review September 16, 2024 15:45
@@ -18,7 +18,7 @@ data:
username {{ .Values.config.username }}
group root-lr
group cisco-support
password {{ .Values.config.password }}
secret {{ include "xrd.hasSecret" . }}
Copy link
Contributor

Choose a reason for hiding this comment

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

RP/0/RP0/CPU0:ios(config-un)#secret ?
  0     Specifies a cleartext password will follow
  10    Specifies that SHA512-based password will follow
  8     Specifies that SHA256-based password will follow
  9     Specifies that Scrypt-based password will follow
  LINE  The cleartext user password

Given that we are using the LINE version of the secret configuration - i.e. the user enters a cleartext password - is there any need to deprecated .Values.config.password? We can just change this to configure secret rather than 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.

My feeling was that it could be confusing to have the word "password" in Helm when the actual config is "secret", given that password also exists as config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On reflection, I don't think the complication of allowing both is worth it, have changed PR to just use secret.

@ebeaty-cisco ebeaty-cisco merged commit bf56a60 into main Sep 24, 2024
2 checks passed
ebeaty-cisco added a commit that referenced this pull request Dec 19, 2024

Co-authored-by: ebeaty-cisco <[email protected]>
ebeaty-cisco added a commit that referenced this pull request Dec 19, 2024
…P and vRouter MGMT interfaces (#21)

* Add additional cpuset options (#13)

* Fix cpu count env var error (#14)

* just change password to secret (#15)

* added sriov for cp interfaces

* added tests that XR_NETWORK_STATUS_ANNOTATION_PATH is not present

* added sriov for cp mgmt interfaces

* sriov mgmt interfaces added for vrouter

* fixed anySRIOV bug

* small ut fix

* markups

---------

Co-authored-by: ebeaty-cisco <[email protected]>
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.

2 participants