search

mozilla / ssl-config-generator

Mozilla SSL Configuration Generator
https://ssl-config.mozilla.org/
Mozilla Public License 2.0
374 stars 60 forks source link

Replace config name with feature check in go/caddy conditions #262

Closed gstrauss closed 5 days ago

gstrauss commented 1 month ago

Replace form.config in template conditionals Instead, prefer to check output.protocols in template conditionals.

gstrauss commented 1 month ago

Aside: awselb.hbs references PolicyName: Mozilla-{{form.config}}-v5-0. Should we open a separate issue to update this? Do you happen to know what needs to happen to update the two references in that file to Mozilla-{{form.config}}-v5-7?

janbrasna commented 1 month ago

For a good laugh here's what worked: https://github.com/janbrasna/ssl-config-generator/commit/9912704d6c2a13f115204d9732a068f35314b700 (so I may try looking into why sstls is not available on the output, rather than anything similar;D — or, ofc, exposing the version from sstls to output or via form if we want it there anyways…) — so yeah, noticed it before and tried to look for a quick fix, but will need arch decision first where else we might need it, and add it there properly.

EDIT: the version is in the data, it just wasn't being stringified for text replacement — I'll include that in the next AWS update.

gstrauss commented 1 month ago

For a good laugh here's what worked: janbrasna@9912704 (so I may try looking into why sstls is not available on the output, rather than anything similar;D — or, ofc, exposing the version from sstls to output or via form if we want it there anyways…) — so yeah, noticed it before and tried to look for a quick fix, but will need arch decision first where else we might need it, and add it there properly.

Not pretty code, but if it works, why not? Short-term, I'd prefer to simply change "5.0" to "5.7" and revisit later. Longer-term, I think I'd prefer having state.js use javascript to export a variable with the extracted value rather than the contortions in HandlebarJS templates.

In any case, awselb.hbs is not referencing {{form.config}} in a template conditional, so is not in scope for this PR. The title of this PR ("... in go/caddy conditions") also excludes awselb.

Are the proposed code changes in this PR ready for merge?

gstrauss commented 5 days ago

On Oct 13 in https://github.com/mozilla/ssl-config-generator/pull/262#issuecomment-2408871266 above, I wrote:

Are the proposed code changes in this PR ready for merge?

You marked the comment as Resolved.

A month has passed. I am merging this PR. @janbrasna this is an example of you not being able to reliably make progress on small, well-scoped changes (or to properly flag the PR with data why the PR is blocked). Note: I created this PR specifically to address your feedback in another PR.