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

Fix incorrect logic in resolve_networks function #6589

Closed
3 tasks done
shurkys opened this issue May 21, 2024 · 0 comments
Closed
3 tasks done

Fix incorrect logic in resolve_networks function #6589

shurkys opened this issue May 21, 2024 · 0 comments

Comments

@shurkys
Copy link

shurkys commented May 21, 2024

Description
The resolve_networks function contains incorrect logic for replacing network variable placeholders with their corresponding network IDs. The current implementation uses the first key in the network hash, which can lead to incorrect substitutions if the key order does not match.

To Reproduce
Steps to reproduce the behavior:

  1. Create a template with vm_template_contents containing network variable placeholders (e.g., $CUSTOM1_VAR).
  2. Define networks_values with multiple network entries.
  3. Execute the resolve_networks function with the template.
  4. Observe that the variable placeholders might be replaced with incorrect network IDs due to key order mismatch.

Example in OpenNebula/terraform-provider-opennebula#527

Expected behavior
The variable placeholders in vm_template_contents should be replaced with the correct network IDs based on the matching keys.

Details

  • Affected Component: OneFlow
  • Hypervisor: KVM
  • Version: Current development version

Additional context
In continuation of PR #6522 (Fix flow key order hardcode), there were changes made in commit 0d1ac37583a43bac6454c3e939fa014e7672c3d2, but the resolve_networks function was not updated. The current logic in the resolve_networks function is incorrect and should be updated for better accuracy.

Current Logic:

role['vm_template_contents'].gsub!(
    '$'+key[0],
    net[net.keys[0]]['id'].to_s
)

In the current implementation, net[net.keys[0]] is used to get the network value. This approach can lead to incorrect results if the order of keys in net does not match key[0].
image
image

Suggested Improvement:

role['vm_template_contents'].gsub!("$#{key[0]}", net[key[0]]['id'].to_s)

By using net[key[0]] directly, we ensure that the correct network value corresponding to key[0] is used, making the function more reliable.

Progress Status

  • Code committed
  • Testing - QA
  • Documentation (Release notes - resolved issues, compatibility, known issues)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants