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

only set randompasswd in graphics template if it's true #535

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

Conversation

nilsding
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for PR followers and do not help prioritize the request

Description

older OpenNebula versions (like 5.12) interpret the RANDOM_PASSWORD as truthy if its value is not empty and therefore try to generate a password, which when using VNC will be too long and not truncated due to another bug that's already been fixed in the latest release.

(yes, I know, we should really update our setup ...)

References

New or Affected Resource(s)

  • opennebula_template

Checklist

  • I have created an issue and I have mentioned it in References
  • My code follows the style guidelines of this project (use go fmt)
  • My changes generate no new warnings or errors
  • I have updated the unit tests and they pass succesfuly
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation (if needed)
  • I have updated the changelog file

older OpenNebula versions (like 5.12) interpret the `RANDOM_PASSWORD`
as truthy if its value is not empty and therefore try to generate a
password, which when using VNC will not be truncated due to another
bug that's already been fixed in the latest release

(yes, I know, we should really update our setup ...)
@frousselet
Copy link
Collaborator

frousselet commented Apr 4, 2024

Hello @nilsding,

Unfortunately we don't support OpenNebula bellow 5.12.
I would suggest you to fork the provider while you are on a version bellow 5.12 and compile your version including your changes on your side.

@frousselet frousselet added the status: won't fix This will not be worked on label Apr 4, 2024
@frousselet frousselet closed this Apr 4, 2024
@nilsding
Copy link
Author

nilsding commented Apr 4, 2024

Hi @frousselet, the readme mentions that OpenNebula ~> 5.12 is still supported 🤔

But the bug fixed by this PR also affects at least >= 6.0, < 6.1.80, as the random_password handling was changed around that time in OpenNebula/one@0edcc7a

@frousselet
Copy link
Collaborator

My bad, I misread 😬
I reopen it

@frousselet frousselet reopened this Apr 4, 2024
@frousselet frousselet removed the status: won't fix This will not be worked on label Apr 4, 2024
Copy link

github-actions bot commented May 5, 2024

This pull request is stale because it has been open for 30 days with no activity and it is not in a milestone. Remove 'status: stale' label or comment, or this will be closed in 5 days.

@akurz
Copy link

akurz commented Jun 13, 2024

Hi @frousselet, looks like this PR was "forgotten" - would you be so kind and reopen it, thx!

@treywelsh
Copy link
Collaborator

treywelsh commented Aug 29, 2024

hi, sorry for the late reply, instead of modifying the default behavior for all the releases, could you check the version instead and add an if - else to handle your specific case ?

Here's an example: https://github.com/OpenNebula/terraform-provider-opennebula/blob/master/opennebula/resource_opennebula_marketplace_app.go#L180

Then rebase your PR, add a line in the CHANGELOG and I'll review it

Copy link

This pull request is stale because it has been open for 30 days with no activity and it is not in a milestone. Remove 'status: stale' label or comment, or this will be closed in 5 days.

@treywelsh
Copy link
Collaborator

I remove the stale label to let it open some more days as it took me a bit of time to review it

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.

4 participants