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

policy: improve target block policy parsing and fix panics. #100

Closed
wants to merge 1 commit into from

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Apr 21, 2020

The target block of a scaling policy is opaque to Nomad and does
not go through any validation during job registration. It can
result in malformed but valid HCL being read by the autoscaler. It
is its responsibility to parse this carefully, avoiding panics
and returning sensible errors where possible.

This fix initially targeted a panic which occurred if the name
param of a target block was blank where the block also included
a config section. When writing tests to cover this, a number of
other cases were uncovered and the scope of the change altered to
accommodate these.

Follow up issues have been raised to address other aspects of
policy parsing: #99 and #98

closes #71

The target block of a scaling policy is opaque to Nomad and does
not go through any validation during job registration. It can
result in malformed but valid HCL being read by the autoscaler. It
is its responsibility to parse this carefully, avoiding panics
and returing sensible errors where possible.

This fix initially targeted a panic which occured if the name
param of a target block was blank where the block also included
a config section. When writing tests to cover this, a number of
other cases were uncovered and the scope of the change altered to
accomodate these.
@jrasell jrasell added the bug label Apr 21, 2020
@jrasell jrasell self-assigned this Apr 21, 2020
@jrasell jrasell marked this pull request as draft April 21, 2020 16:34
@lgfa29
Copy link
Contributor

lgfa29 commented May 4, 2020

Closed in favor of #114

@lgfa29 lgfa29 closed this May 4, 2020
@lgfa29 lgfa29 deleted the gh-71 branch May 4, 2020 17:18
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.

Target parsing panics when name is not set
2 participants