search

codeout / junoser

PEG parser for JUNOS configuration.
MIT License
69 stars 11 forks source link

set文羅列とdelete文羅列を入力すると、deleteを適用したset文羅列にできる差分 #8

Closed shogosanz closed 6 years ago

shogosanz commented 6 years ago

大変僭越ながらJunoserの機能を実装したので、ご査収いただければ幸いです。

使い方

junoser --delete CONFIG_FILE

(01:06:45)20202461n: junoser$cat config 
set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 1 family inet mtu 1000
set interfaces em0 unit 1 family inet6
set interfaces em1 unit 0
set interfaces em2
delete interfaces em0 unit 0 family inet address 1.1.1.1/32
delete interfaces em0 unit 1
(01:06:48)20202461n: junoser$ruby exe/junoser --delete config 
set interfaces em0 unit 0 family inet
set interfaces em1 unit 0
set interfaces em2

概要

Delete文は既存のConfigによって適用のされ方が異なるという性質があります。 これは、手順書の自動作成においてこれを判断をするために人間の手が介入させなければならないというデメリットを生じさせてしまうものです。

例：

set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 1 family inet address 1.1.1.2/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32
↓
set interfaces em0 unit 0 family inet
set interfaces em0 unit 1 family inet address 1.1.1.2/32
set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 0 family inet address 1.1.1.2/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32
↓
set interfaces em0 unit 0 family inet address 1.1.1.2/32

Junoserは「機器にログインせずに作業が出来るようにする」という目的を元に作られていると認識しています。この機能は、この目的に合致するのではないかと考えています。

もしこの差分がマージされた場合、以下のようなメリットが考えられます。

実装

大まかなフローは以下になります。

入力
↓
set文とdelete文を分離
↓
set文をstructureに変換。
↓
さらにこれをhashに変換。(新実装部分)
↓
delete文を一行ずつhashにし、set文のhashと比較して、削除する。(新実装部分)
↓
最終的なset文のhashをstructureに変換 (新実装部分)
↓
structureをset形式に変換
↓
出力

関数

apply

引数: io_or_string 戻り値: delete文適用後のset文羅列

set文とdelete文を分解し、いくつかの関数を呼び出し、処理する、メインのフローです。

struct_to_hash

引数: struct 戻り値: hash

ConfigのStructureをHashに変換します。 後々Delete文を適用しやすくするためのものです。 内部では、struct_to_first_hashとhash_value_to_hashを用いています。 例

interface em0{
        unit 0{
                family inet;
        }
}

↓

{"interface em0" => {"unit 0" => {"family inet"=> {} }}}

hash_value_to_hash

引数: hash 戻り値: hash

入力されたHashのValueたちをさらにstruct_to_first_hashでHash化していく再帰関数です。

struct_to_first_hash

引数: struct 戻り値: hash

一番外側にある構造だけをHashにし、内側はすべて文字列として扱ってしまうものです。 例

interface em0{        unit 0{                family inet;        }}

↓

{"interface em0" => "unit0{                family inet;        }"}

apply_delete

引数: set文のhash , delete文のhash 戻り値: 適用後のset文のhash

delete文は一行ずつ適用されるため、ret_first_key_and_value関数を用いて、hashの一番最初のKeyとValueを取得してくるものです。これによって、Set文のKeyと比較し、正しく削除します。

hash_to_struct

引数: hash 戻り値: struct

最後にhashをStructに直すためのものです。 hash_to_struct_iterでジェネレートされるStringを追加していくだけのものです。

例

{"interface em0" => {"unit 0" => {"family inet"=> {} }}}

↓

interface em0{
unit 0{
family inet;
}
}

hash_to_struct_iter

引数: hash 戻り値: string(structの部分)

Hashを少しずつStringになおしていき、最終的にStructureにします。 これを実現するためのジェネレータとなる再帰関数です。

ret_first_key_and_value

引数: hash 戻り値: key,value

hashの一番最初のKeyとValueを取得します。Delete文は一文ずつ用いるので、最初だけで全てを取得したことになります。

codeout commented 6 years ago

パッチありがとうございます！

Delete文は既存のConfigによって適用のされ方が異なるという性質があります。 これは、手順書の自動作成においてこれを判断をするために人間の手が介入させなければならないというデメリットを生じさせてしまうものです。

同じ問題意識を持っています。が、その問題は少々大きいため、junoser では「コンフィグの生成には関与しない。パースするだけ」にフォーカスしています。

Junoserは「機器にログインせずに作業が出来るようにする」という目的を元に作られていると認識しています

圧縮された最適なコンフィグを出力するのは 既存の機能にはないものですので、exe/ 配下の別コマンドにするという方向性はいかがでしょうか？

いずれにせよ、いい名前を思いつく必要はありそうですよね。

入出力パターン

  1. 構造化コンフィグ + delete → 構造化コンフィグを出力
  2. 構造化コンフィグ + delete → | display set コンフィグを出力
  3. | display set コンフィグ + delete → 構造化コンフィグを出力
  4. | display set コンフィグ + delete → | display set コンフィグを出力

今回 4 に特化しています。

順序によって出力が変わるSyntax パターン

  1. delete + set
  2. deactivate + activate
  3. replace
  4. rename
  5. insert
  6. protect + unprotect
  7. ...

今回 1 に特化しています。

今回実装いただいたものにいい名前をつけられるのであれば、これは便利そうかなと思いました。

shogosanz commented 6 years ago

レビューありがとうございます。

junoser では「コンフィグの生成には関与しない。パースするだけ」にフォーカスしています。 exe/ 配下の別コマンドにするという方向性はいかがでしょうか？

なるほど。像を理解しました。 すると確かにオプションとするのは変ですね。 junoser-delete CONFIG などとなるほうが良いですね。(例ですが)

いずれにせよ、いい名前を思いつく必要はありそうですよね。

名前に関しても気持ち悪いところはあり、-d は既にあるのに、 --deleteとか名付けちゃってるとか delete以外にも、もう少し明瞭な表現でapply-deleteとかも考えていました。 確かにrename,replaceなどの将来的な実装を考えると、より良い命名をする必要がありますね。

他にも、例えば

-- config --
set interfaces em0 unit 0 family inet address 1.1.1.1/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32
-- config --
>$ junoser-delete CONFIG

や

-- config --
set interfaces em1 unit 0 family inet address 1.1.1.2/32
rename interfaces em1 to em2
-- config --
>$ junoser-rename CONFIG

や

-- config --
set interfaces em3 unit 0 family inet mtu 1500
copy interfaces em3 to em4
-- config --
>$ junoser-copy CONFIG

・ ・ ・ このようにコマンドを分けるより、何が混ざっていようと

-- config --
set interfaces em0 unit 0 family inet address 1.1.1.1/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em1 unit 0 family inet address 1.1.1.2/32
rename interfaces em1 to em2
set interfaces em3 unit 0 family inet mtu 1500
copy interfaces em3 to em4
-- config --
>$ junoser-compact CONFIG

↓

set interfaces em0 unit 0 family inet
set interfaces em2 unit 0 family inet address 1.1.1.2/32
set interfaces em3 unit 0 family inet mtu 1500
set interfaces em4 unit 0 family inet mtu 1500

で処理できる、万能なコマンドがあっても良いと考えています。(compactとかoptimとか)

ただ、deactivate,activateを除くいずれのsyntaxパターンにおいても、set&deleteだけで表現可能だという認識なので、deleteコマンドのみ独立性があっても良いかなとも考えています。 この点に関しては意見を頂けると有難いです。

rename,copy,insert・・・などのコマンドの実装について

この差分では、構造化configを、rubyとして扱いやすくkey参照を可能にする順序性データ構造であるhashにしていますので、これによって他のコマンドの実装を容易にすると考えています。

パターンについて

入出力パターンの1,2,3,4にそれぞれ対応するのは数行追加するだけで行けそうですが(そもそも、このような場合についてはユーザーがパイプで渡せば良いと考えていました)、順序によって出力が変わるsyntaxパターンについてはさらなる検討が必要ですね(特に、この実装だとdeactivate+activate関連が考慮されていませんでした・・・これは認識不足でした。一時的に退避させ、生成後に追加するような構造にしようかと思います)

特に、delete文set文間においても適用順序で出力が異なる件については、「set文を既存のConfig、delete文は今から注入する」というベースで考えていたので、「いかなる順序だろうとset文の集合に対してdelete文を一方的に適用すれば良い」としてきましたが、この点は要修正としたいと思います。(順序に意味が生じるように。) つまり、

delete interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 0 family inet address 1.1.1.1/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32

これらは明確に区別しようと考えています。 実装レベルでは、上から順番に適用していくという処理にしていくつもりです。 (set文間、delete文間においては順序は関係ないため、順序を考慮せずに追加しますが。)

疑問点

一般論として、手順書を生成する前にどのような作業を行っているのかというのがいまいち把握出来ておりません。 私が知っているのは弊社の場合のみで、「作業終了後にshow configuration | display setしたらこのような出力になっているべき」というconfigを管理しているという事情があるため、replaceやrenameの出現は想定外でした。(どのようなConfigであろうと、delete文とset文だけで表現可能なはずなため)

しかし、多くの人にとって使いやすく有るべきだと考えるので、この点については、より汎用的な設計にしたいと考えます。

まとめ 追加したい・修正したい点

将来的に追加したい機能

codeout commented 6 years ago

名前はさておき、

で処理できる、万能なコマンドがあっても良いと考えています。(compactとかoptimとか)

万能コマンドを用意しておいて「いまは delete + set だけの実装ですよ」のほうがよいですかね。junoser コマンドとは分けるが、それを使う限りユーザーは呼び出し方法を変えずに機能実装の恩恵を受けられそうです。

この場合やりたいのは「現状のコンフィグと変化を与えるコマンド(deleteとか) を入力して、junos が吐くであろう変化後を出力する」でしょうか。変化を与えるコマンドは運用者によってまちまちのはずです。

あとで内部実装も拝見してコメントしますね。

shogosanz commented 6 years ago

万能コマンドを用意しておいて「いまは delete + set だけの実装ですよ」のほうがよいですかね

それを使う限りユーザーは呼び出し方法を変えずに

なるほど。 この機能を使うユーザーが現れたとき、次のアップデートによるユーザー側の修正を極力抑えるべきだということですね。

この場合やりたいのは「現状のコンフィグと変化を与えるコマンド(deleteとか) を入力して、junos が吐くであろう変化後を出力する」でしょうか。変化を与えるコマンドは運用者によってまちまちのはずです。

仰る通りです。 だから、ある使い方だけに対応してしまっては、あまり意味の無い自己満足になってしまうと思うのです。

あとで内部実装も拝見してコメントしますね。

ありがとうございます！ 宜しくお願い致します。 既に発覚している修正すべき点のみ、以下に記します。

十分なdelete文の例

set interface em0 unit 0
delete interface em0 unit 0 family inet

これは終了されずに、実行が続けられるべきだと考えますので、要修正とします。 begin-rescueで囲むつもりです。

shogosanz commented 6 years ago

内部実装を読んでいただいているところ修正申し訳ありません。 struct_to_first_hash の処理がなんだか気持ち悪く読みにくいのではないかと思い、下図の状態遷移図となるように実装し直しました。少しはマシになって読みやすくなったかなと思います。 statemachine

任意の状態で{ }以外([^(\{|\})])は状態を維持します。 状態2以上(実装ではcase文におけるelse)は、2の状態が繰り返されるだけであります。

shogosanz commented 6 years ago

いくつか修正と追加を致しました。 ブランチを分けようとも思ったのですが、いくつか共通したファイルを編集している上に機能が似通っているため、コンフリクトを避ける意味でも同じブランチにPushしました。 なお、delete.rbのコードに関してはほとんど手を加えておりません。

junoser-compareコマンドに関わる差分

例

Config1

set interfaces em0 unit 0 family inet mtu 1500
set interfaces em0 unit 0 family inet6
set interfaces em1 unit 1
set interfaces em2
set interfaces em100 unit 0 family inet mtu 1500
set interfaces em100 unit 0 family inet6

Config2

set interfaces em0 unit 0 family inet mtu 1500
set interfaces em1
set interfaces em2 unit 1 family inet
set interfaces em3

結果

delete interfaces em0 unit 0 family inet6
delete interfaces em1 unit 1
delete interfaces em100

set interfaces em2 unit 1 family inet
set interfaces em3

hash_cmp関数

hashを比較し、差を実現するset文あるいはdelete文の、もととなるhashを返します。

including関数

Configの包含関係を調べます。あるConfigがあるConfigの一部であるか否かをhashを用いて判定しています。 例

{"interface em0" =>{"unit 0"=>{"family inet" =>{}}}}
は
{"interface em0" =>{"unit 0"=>{"family inet" =>{}}, "unit 1"=>{}}} 
の部分。
codeout commented 6 years ago

いろいろ検討いただいているのに遅くてすみません。

内部実装のところ、一点コメント差し上げたく。

おさらい: delete の適用がむずかしい理由

Delete文は既存のConfigによって適用のされ方が異なるという性質があります。

この一点かなと思っています。具体的には

1: set interfaces em0 unit 0 family inet address 1.1.1.1/32
2: delete interfaces em0 unit 0 family inet address 1.1.1.1/32

で set interfaces em0 unit 0 family inet を残さないといけないところ。 junoser でパースして残す範囲を検出しなければなりません。 検出さえすれば、文字列操作に落とし込めそうと思いました。

問題: パフォーマンス

現在速いとは言い難いので、なるべくパースする部分を減らしたいところ。

提案: 次のような戦略ではいかがでしょうか？

  1. set 文を見つけたら、何もしない (| display set を仮定して)
  2. delete 文を見つけたら、文字列操作のための正規表現をつくり、1 に適用する
  3. uniq なりの後処理

set 文をparse しないことを狙っています

ヒント: パースした段階の構文木を取ってくるのは可能

require 'junoser'

parser = Junoser::Parser.new
pp parser.parse('set interfaces em0 unit 0 family inet address 1.1.1.1/32')
junoser $ ruby -rpp foo.rb
{:config=>
  {:label=>"interfaces"@4,
   :child=>
    {:label=>{:arg=>"em0"@15},
     :child=>
      {:label=>{:statement=>"unit"@19, :argument=>"0"@24},
       :child=>
        {:label=>"family"@26,
         :child=>
          {:label=>"inet"@33,
           :child=>
            {:label=>
              {:statement=>"address"@38, :argument=>"1.1.1.1/32"@46}}}}}}}}

junoser-compare について

ありがとうございます！ しかしながら話が発散するため、このPR では「delete を適用するやつ」に限定させてください。

あと、名前は junoser-squash はどうでしょうか。

codeout commented 6 years ago

実装例: delete をパースして変換して正規表現にする例

https://gist.github.com/codeout/a5a47e4143c2473767a509d25ddcb036

set interfaces em0 unit 0 family inet
delete interfaces em0 unit 0 family inet address 1.1.1.1/32

のように、消す対象そのものがなかったらどうふるまうべきか、など検討は若干あまいかもです

shogosanz commented 6 years ago

おお! ありがとうございます!

ヒント: パースした段階の構文木を取ってくるのは可能

これは夢が広がりました！良いヒントをありがとうございます。

現在速いとは言い難いので、なるべくパースする部分を減らしたいところ。 提案: 次のような戦略ではいかがでしょうか？ set 文を見つけたら、何もしない (| display set を仮定して) delete 文を見つけたら、文字列操作のための正規表現をつくり、1 に適用する uniq なりの後処理

なるほど、これは良い案だと思います。 delete文のみをパースすればset文をパースするまでもなく判断出来ることを利用するのですね。 方針については全くの同意です。

以下細かい点について述べます。

例えば、頂いた実装例と共に以下のような例を考えます。

例

まず、実装例有難うございます。 実装例に対してこのような例を考えると、 入力

set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 0 family inet mtu 1500
set interfaces em0 unit 1 family inet address 1.1.1.2/32
delete interfaces em0 unit 0 family inet address 1.1.1.1/32

↓出力

set interfaces em0 unit 0 family inet 
set interfaces em0 unit 1 family inet address 1.1.1.2/32

となってしまいます。

この本質は、transformからのpatternの生成にあります。

to_delete = parser.parse('set interfaces em0 unit 0 family inet address 1.1.1.1/32')
@transformer.apply(to_delete)
↓
(interfaces\\ em0\\ unit\\ 0\\ family\\ inet\\ ).*

最後の子が消えてしまっているところに、情報の欠落があります。

改善点

以下のような実装はどうでしょう。

例

set interfaces em0 unit 0 family inet address 1.1.1.1/32
set interfaces em0 unit 0 family inet mtu 1500
set interfaces em0 unit 0 family inet6 address ::1/128
delete interfaces em0 unit 0 family inet

まず、

to_delete = parser.parse('set interfaces em0 unit 0 family inet')

によって、delete文に構文的な意味を与えます。

うまいこと変換し、 こんな正規表現を生成します。(アドレスに用いられる.や/はうまいことエスケープするとして)

(interfaces em0 unit 0 family inet).*

そして、これを.sub!(/#{pattern}/) { $1 }します。 同時に、これに該当したset文は1番下の子を消します。

全てのset文に対して適用が終わると、以下のようになるはずです。

set interfaces em0 unit 0
set interfaces em0 unit 0
set interfaces em0 unit 0 family inet6 address ::1/128

その後、ある文の部分になるようなものは消します。(set文は長いほうが偉い)

set interfaces em0 unit 0 family inet address 1.1.1.2/32

以上のようなアルゴリズムはどうでしょうか。

junoser-compareについて

junoser-compare に関しては、別PRに致します。 これも、ヒントを使えばもっとシンプルかつ低コストなコードに出来そうな気がします。

名前に関して

junoser-squash はどうでしょうか。

とてもしっくりきます！ 特に、gitなどユーザーが親しんでいるツールと同じ言い回しを用いるのは良い選択だと思います。

codeout commented 6 years ago

なるほど！ いいですね。

squash という趣旨からすると

set interfaces em0 unit 0
set interfaces em0 unit 0
set interfaces em0 unit 0 family inet6 address ::1/128

は最終行だけでいい。 これ愚直にやると O(n^2) の計算量になりますが、そこまでの価値があるかという点ですかね。

codeout commented 6 years ago

最後の子が消えてしまっているところに、情報の欠落があります。

は、"(interfaces em0 unit\\ 1 family inet )address\\ 1\\.1\\.1\\.1/32" こういう正規表現にすればよいだけなので微修正でいけそう。

https://gist.github.com/codeout/a5a47e4143c2473767a509d25ddcb036 更新しました

codeout commented 6 years ago

悩み: squash はソートを含むか

1: set interfaces em0 unit 0 family inet6 address ::1/128
2: set interfaces em0 unit 0

なら1行にしていいと思いますが、

a: set interfaces em0 unit 0
b: set interfaces em0 unit 0 family inet6 address ::1/128

を1行にしていいかは迷いますね。 a行までの時点ではその行に意味がありますので。 b行が現れて初めて、a行の意味がなくなります。 ( 対して、一番上の例の2行目は全く意味がない )

squash を「全て入れた後の設定をsquash」とするか「1行ずつ入れて行って、効果のない行だけ削る」とするかによりそうです。

趣旨としては前者に近く、であればソートしちゃったほうが文字列処理がラクそうです。

codeout commented 6 years ago

時間がたってしまい申し訳ありません。

11 を拝見することにし、#8 はclose してもOK、でしょうか？

shogosanz commented 6 years ago

こちらこそ申し訳ありません。

11 を拝見することにし、#8 はclose してもOK、でしょうか？

はい。お願いいたします！