bind-formula icon indicating copy to clipboard operation
bind-formula copied to clipboard

DRY: configured_zones, available_zones

Open aphor opened this issue 7 years ago • 10 comments

Convention over configuration:

If a zone is defined in a pillar, I want the salt states to ensure it is declared in the conf file and also the records are declared in the zone file.

@ppieprzycki thought it was a good idea for Debian in 2947dde649e480091f99552bf7479a0a15c1fb36

It's probably a good idea in general.

aphor avatar Sep 29 '18 00:09 aphor

Hello @aphor ,

Have you tried to use available_zones ? It should declare the zone and manage its content as well.

aanriot avatar Oct 01 '18 07:10 aanriot

Zone configuration is templated in bind/files/named.conf.local.jinja and only seems to emits zones found by iterating over configured_zones https://github.com/saltstack-formulas/bind-formula/blob/a96669c191f286214527a4991e28e1b5dc4241f0/bind/files/named.conf.local.jinja#L80

aphor avatar Oct 03 '18 13:10 aphor

Why is there even a distinction between configured_zones and available_zones ?

aphor avatar Oct 03 '18 13:10 aphor

Hi @aphor, AFAICS, it dates way back to ~2015. But there's some documentation on the usage of these 2 variables here

TL; RL: you use configured_zones to declare which zones are being served by this particular nameserver, being them master, slave, zone files managed or not by this formula or some other way, etc., etc. And then, you use available_zones to populate the zone files.

Hope this helps.

javierbertoli avatar Oct 03 '18 13:10 javierbertoli

That helps, but seems over complicated.

I think the behavior implemented for Debian in 2947dde was a move in the right direction.

Would anyone object to making the formula behave as @aanriot suggested?

aphor avatar Oct 03 '18 19:10 aphor

This issue, #120 and PR #121 are all suggestions that this formula needs some refactoring to DRY/simplify the logic on zone/s - view/s management.

I think that something like (pseudo here, not necessarily correct):

views:
  - view1
  - view2
  - viewN

zones:
  zone1:
    enabled: true
    external_file: /path/or/ref/to/source/of/file
    records:
        all the records code
    view:         # If present, zone will be exposed in the corresponding view
       - view1
  zone2:
    enabled: false     # to keep the file, but disable the zone to be served
    external_file: /path/or/ref/to/source/of/file  # if present, file with zone records will be managed externally
  zoneN:

Maybe something like this covers all cases we need to consider?

javierbertoli avatar Dec 28 '18 15:12 javierbertoli

This issue, #120 and PR #121 are all suggestions that this formula needs some refactoring to DRY/simplify the logic on zone/s - view/s management.

Indeed @javierbertoli , the formula has become too much complicated. Though, I think that we should opt for bind-ng states and pillars, like https://github.com/saltstack-formulas/nginx-formula did, to avoid breaking existing setups.

I think that something like (pseudo here, not necessarily correct):

views:
  - view1
  - view2
  - viewN

zones:
  zone1:
    enabled: true
    external_file: /path/or/ref/to/source/of/file

As the file path is always needed, we may want to use instead:

zones:
  zone1:
    enabled: true
    zonefile: /path/or/ref/to/source/of/file
    managed (or external): true
records:
    all the records code
view:         # If present, zone will be exposed in the corresponding view
   - view1

zone2: enabled: false # to keep the file, but disable the zone to be served external_file: /path/or/ref/to/source/of/file # if present, file with zone records will be managed externally zoneN:


Maybe something like this covers all cases we need to consider?

It seems to, if we keep a way to detect the file path when it's not defined.

aanriot avatar Dec 31 '18 08:12 aanriot

Indeed @javierbertoli , the formula has become too much complicated. Though, I think that we should opt for bind-ng states and pillars, like https://github.com/saltstack-formulas/nginx-formula did, to avoid breaking existing setups.

Please, if possible, don't start a new -ng branching situation :smile:

We had a lengthy discussion about that issue a few days back and the consensus was to use semantic versioning and adding proper documentation in the formulas, so we are trying to start moving formulas that way.

javierbertoli avatar Dec 31 '18 12:12 javierbertoli

I missed this discussion, great news @javierbertoli.

aanriot avatar Dec 31 '18 12:12 aanriot

Hello,

We were talking about this issue with a colleague and a new pillar key could be a a way to avoid breaking existing setups:

zones:
  zone1:
    managed: true

@javierbertoli , I know that it doesn't solve the structural problem of the formula but it would at least allow to use managed: false if we don't want bind to manage the zone file.

Any feelings?

Best regards,

aanriot avatar Apr 26 '19 13:04 aanriot