Skip to content

fix: reject unsupported hypervisor values in host resource validation - #337

Open
nagaboinaramgopal wants to merge 1 commit into
apache:mainfrom
nagaboinaramgopal:fix/host-hypervisor-validator
Open

fix: reject unsupported hypervisor values in host resource validation#337
nagaboinaramgopal wants to merge 1 commit into
apache:mainfrom
nagaboinaramgopal:fix/host-hypervisor-validator

Conversation

@nagaboinaramgopal

Copy link
Copy Markdown

Description

The hypervisor ValidateFunc on the cloudstack_host resource used sort.SearchStrings as if it returned a "found" flag, but sort.SearchStrings returns the insertion index (0..len). The check >= len(validHypervisors) therefore only rejected a value that sorts after every supported entry, so unsupported values such as foo, docker, esxi, or kvm2 passed plan-time validation and failed later with an opaque server-side error at apply.

Fixed by treating a value as valid only when the element at the returned index actually equals it, which is the standard sort.SearchStrings idiom.

Testing

Added a unit test TestResourceCloudStackHostHypervisorValidation asserting the supported hypervisors are accepted and unsupported values are rejected. It needs no live CloudStack:

go test ./cloudstack/ -run TestResourceCloudStackHostHypervisorValidation

The test fails against the current code (bad values are accepted) and passes with the fix.

The hypervisor ValidateFunc used sort.SearchStrings as if it returned a
found flag, but it returns the insertion index (0..len). The check only
rejected a value that sorts after every supported entry, so typos such as
foo, docker or esxi passed plan-time validation and failed later with an
opaque server-side error at apply. Treat a value as valid only when the
element at the returned index equals it. Adds a unit test for the validator.

Signed-off-by: Ramgopal Nagaboina <ramgopal.nagaboina.dev@gmail.com>
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.

1 participant