tinkerbell / tink

Workflow Engine for provisioning Bare Metal
https://tinkerbell.org
Apache License 2.0
913 stars 134 forks source link

Sanitize hardwares entry when creating a workflow #115

Closed invidian closed 4 years ago

invidian commented 4 years ago

it seems if one creates a workflow with hardware index containing -, it is not being picked up by the template properly. Example:

1 ✗ (35.2ms) 11:05:55 invidian@dellxps15mateusz ~/data/workspaces/tinkerbell $ tink template get d9b6027f-fd9f-40fc-bfb0-1513f5a9a9af | head -n 6
version: '0.1'
name: flatcar-install
global_timeout: 1800
tasks:
- name: "flatcar-install"
  worker: "{{.test-controller-0}}"
0 ✓ (32.2ms) 11:06:43 invidian@dellxps15mateusz ~/data/workspaces/tinkerbell $ tink workflow create -t d9b6027f-fd9f-40fc-bfb0-1513f5a9a9af -r '{"test-controller-0": "192.168.50.10"}'
Created Workflow:  a11be694-951b-404d-8c11-d5ef245481d3
0 ✓ (37.0ms) 11:06:58 invidian@dellxps15mateusz ~/data/workspaces/tinkerbell $ tink workflow get a11be694-951b-404d-8c11-d5ef245481d3

0 ✓ (32.2ms) 11:07:04 invidian@dellxps15mateusz ~/data/workspaces/tinkerbell $

When _ is used instead of -, everything is OK.

parauliya commented 4 years ago

Hi @invidian , It is mentioned in the docs here https://github.com/tinkerbell/tink/blob/master/docs/cli/workflow.md that the key can only container letter, numbers and underscores only.

invidian commented 4 years ago

Thanks for the hint @parauliya. In my opinion, this should be validated server-side, so the user gets right error message when such input is used. What do you think?

parauliya commented 4 years ago

@invidian , This can be the solution but rather I would like to let the user know about this restriction so that user can provide the correct key with supported letters. I will add a Note in the website at the Template and Workflow section for now.

gauravgahlot commented 4 years ago

Closing this issue, as the relevant docs have been updated in PR #48