2022-12-14

mackerel-agent configtest によるチェック強化の実装について

Go Mackerel

Mackerel Advent Calendar 2022 15日目です。

mackerel-agent configtestによるチェック機能を強化したので実装を軽く紹介します。
github.com
mackerel.io

何が変わったのか

次のようなmackerel-agent-sample.confを作成し、mackerel-agent configtestを実行する。

apikey = "abcdefg"
podfile = "/path/to/pidfile"

[foo]
command = "bar"
 
[plugin.checks.incorrect1]
command = "test command"
action = { command = "test command", user = "test user", en = { TEST_KEY = "VALUE_1" } }

[plugins.checks.incorrect2]
command = "test command"

[plugin.check.incorrect3]
command = "test command"

強化前は、Syntax OKとなる。

$ mackerel-agent configtest -conf /usr/local/etc/mackerel-agent-sample.conf
/usr/local/etc/mackerel-agent-sample.conf Syntax OK

強化後は、各設定項目がtypoで無効になっていたら警告を出してくれる。

$ mackerel-agent configtest -conf /usr/local/etc/mackerel-agent-sample.conf
[WARNING] foo is unexpected key. Did you mean root ?
[WARNING] plugin.check.incorrect3 is unexpected key. Did you mean plugin.checks.incorrect3 ?
[WARNING] plugin.checks.incorrect1.action.en is unexpected key. Did you mean plugin.checks.incorrect1.action.env ?
[WARNING] plugins is unexpected key. Did you mean plugin ?
[WARNING] podfile is unexpected key. Did you mean pidfile ?

実装

候補の列挙

候補とは、Did you mean ***の***のことで、各設定項目に該当する。
コードではconfig/config.goのConfig構造体に列挙されている。

type Config struct {
	Apibase       string
        ...
        DisplayName   string        `toml:"display_name"`
        ...
        HostStatus    HostStatus    `toml:"host_status" conf:"parent"`
        ...
        HostIDStorage   HostIDStorage   `conf:"ignore"`
        ...
}

Apibaseのように、タグがついていない場合、フィールド名の先頭を小文字にして追加する。
DisplayNameのように、tomlタグがついている場合、その値を追加する。
HostStatusのように、フィールドの型が構造体の場合、parentタグをつけて候補を再帰的に取得する(ここだとon_start, on_stop)。
HostIDStorageのように、設定ファイルに書き込まれることがない場合、ignoreタグをつけてスキップする。

実装: mackerel-agent/validate.go at 5a2668daac02a5e2a406d22ddd23af6cca32e7be · mackerelio/mackerel-agent · GitHub

typoキーの検出

typoキーは、BurntSushi/toml#MetaData.Undecodedを用いて検出できる。

上記の設定ファイルの例では、次のように検出できている。ソートしているのは後の処理のため。

mackerel-agent/validate.go at 5a2668daac02a5e2a406d22ddd23af6cca32e7be · mackerelio/mackerel-agent · GitHub

func ValidateConfigFile(file string) ([]UnexpectedKey, error) {
    config := &Config{}
    md, err := toml.DecodeFile(file, config)
    ...
    undecodedKeys := md.Undecoded()
    sort.Slice(undecodedKeys, func(i, j int) bool {
	return undecodedKeys[i].String() < undecodedKeys[j].String()
    })

    // [foo foo.command
    //  plugin.checks.incorrect1.action.en plugin.checks.incorrect1.action.en.TEST_KEY
    //  plugins.checks.incorrect2 plugins.checks.incorrect2.command
    //  podfile]
    fmt.Printf("%+v", undecodedKeys)

typoキーの判定

fooの場合、fooは候補として列挙されていないので、typoキーはfoo。
plugin.checks.incorrect1.action.enの場合、pluginが候補として列挙されているので、typoキーはen。
plugins.checks.incorrect2の場合、pluginsは候補として列挙されていないので、typoキーはplugins。
podfileの場合、podfileは候補として列挙されていないので、typoキーはpodfile。

foo.commandは、fooが既に判定されているのでスキップ。
plugin.checks.incorrect1.action.en.TEST_KEYは、plugin.checks.incorrect1.action.enが既に判定されているのでスキップ。
plugins.checks.incorrect2.commandは、plugins.checks.incorrect2が既に判定されているのでスキップ。

スキップしないとContents of suggestion when executing configtest may not be correct · Issue #829 · mackerelio/mackerel-agent · GitHubのような不具合が生じてしまう。

実装: mackerel-agent/validate.go at 5a2668daac02a5e2a406d22ddd23af6cca32e7be · mackerelio/mackerel-agent · GitHub
Config構造体にparentタグがあるかで判定するより、候補として列挙されているかどうかで判定する方が良かったというPR:
refactor(config/validate): use `candidates` instead of `parentConfKeys` by wafuwafu13 · Pull Request #839 · mackerelio/mackerel-agent · GitHub

plugin.typo.fooの検出

プラグインの設定項目は、checks, metrics, metadataでないといけないが、Config構造体ではstringであれば良いので、MetaData.Undecodedでは検出できない。
上記の設定ファイルの例だと、plugin.check.incorrect3が検出されていない。

type Config struct {
   ...
   Plugin map[string]map[string]*PluginConfig `conf:"parent"`
   ....
}

そのため、config.Pluginを別途調べる必要がある。

実装: mackerel-agent/validate.go at 5a2668daac02a5e2a406d22ddd23af6cca32e7be · mackerelio/mackerel-agent · GitHub

候補の判定

agext/levenshteinで、レーベンシュタイン距離を用いて判定する。
Terraformでも使われている。

結果の表示

TOMLの形式がおかしいなど致命的なエラーは赤、typoの警告は黄で表示する。
\x1b[31m *** \x1b[0mのように文字列で指定するとWindowsでうまく表示できないのでfatih/colorを使用した。

展望

今はtypoキーをソートして表示しているけど、余裕があれば設定ファイルの行数を取得してその順番通りにtypoの行数も表示したい。

2022-12-01

Rust学習進捗2022

Rust

フロントエンド界隈のOSSで目立ってきたり、Linuxに取り込まれたり、放ってはおけない言語、それがRust。

1年前くらいに入門したこの本に再び目を通した。
gihyo.jp

実践で書いてみたくなったから awesome-alternatives-in-rust に目を通してテストが足りてなかったjqlにコミット。
github.com
github.com

Denoにもちょこっとコミット。
github.com
wafuwafu13.hatenadiary.com

自分でも何か作りたくなったから mkr の Rust実装 mkrust を作成。基本的なことを一通り終えたから開発が止まっている。
github.com
github.com

Cと比較されることが多いのでC版の mkrc を作成。
github.com

メモリ管理の良さを実感できなかったのでガベージコレクションのアルゴリズムと実装を読んだ。
tatsu-zine.com

GCが搭載されてないといかに大変かがわかった本。
book.mynavi.jp

Do'er Advent Calendar 2022 の1日目でした。

2022-09-26

deno/std/nodeのfs.DirentでisBlockDeviceの判定ができないのはRustに実装がされていないから...ではない

OSS活動 Rust Deno

追記: Rustにis_block_deviceがあって、Denoにはなかった
github.com

fs.readdir(path, options, callback)のoptionsにwithFileTypes: trueを指定すると、fs.Direntが返ってくる。
fs.Direntのdirent.isBlockDevice()を使った例が以下。

$ node
Welcome to Node.js v16.13.1.
Type ".help" for more information.
> const files = fs.readdirSync(".", {withFileTypes: true})
undefined
> files[0].isFile()
true
> files[0].isBlockDevice()
false

しかし、Deno(std/node)では使えない。

$ deno
Deno 1.25.4
exit using ctrl+d or close()
> import {readdirSync} from "https://deno.land/std@0.157.0/node/fs.ts"
undefined
> const files = readdirSync(".", {withFileTypes: true})
undefined
> files[0].isFile()
false
> files[0].isBlockDevice()
Uncaught Error: Not implemented: Deno does not yet support identification of block devices
    at notImplemented (https://deno.land/std@0.157.0/node/_utils.ts:23:9)
    at Dirent.isBlockDevice (https://deno.land/std@0.157.0/node/_fs/_fs_dirent.ts:8:5)
    at <anonymous>:2:10

deno_std/node/_fs/_fs_dirent.tsを見るとnotImplementedとされている。

export default class Dirent {
  constructor(private entry: Deno.DirEntry) {}

  isBlockDevice(): boolean {
    notImplemented("Deno does not yet support identification of block devices");
    return false;
  }
  ...

std/nodeのreaddirSyncは内部的にはDeno.readDirSyncが使われているのでDenoのコードを見にいくとそれっぽい箇所があった。
is_fileがあるから上記の例でfiles[0].isFile()が使えている。

  FsStat {
    is_file: metadata.is_file(),
    is_directory: metadata.is_dir(),
    is_symlink: metadata.file_type().is_symlink(),
...

metadataの参照先はRustなのでRustのコードを見にいくとそれっぽい箇所があった。

impl Metadata {
    #[must_use]
    #[stable(feature = "file_type", since = "1.1.0")]
    pub fn file_type(&self) -> FileType {
        FileType(self.0.file_type())
    }

    #[must_use]
    #[stable(feature = "rust1", since = "1.0.0")]
    pub fn is_dir(&self) -> bool {
        self.file_type().is_dir()
    }

    #[must_use]
    #[stable(feature = "rust1", since = "1.0.0")]
    pub fn is_file(&self) -> bool {
        self.file_type().is_file()
    }

    #[must_use]
    #[stable(feature = "is_symlink", since = "1.58.0")]
    pub fn is_symlink(&self) -> bool {
        self.file_type().is_symlink()
    }
...

ここにis_block_deviceを生やせば解決するはず...だけどメタ的な意味でRustわからん

2022-09-25

JSのオブジェクトの分割代入で既定値が割り当てられるのはundefinedの場合のみ

OSS活動 JavaScript

「JSのオブジェクトの分割代入で既定値が割り当てられるのはundefinedの場合のみ」ということは、MDNのAssigning to new variable names and providing default valuesにAssigned a default value in case the unpacked value is undefined.と書いてあることや、13 ECMAScript Language: Expressionsの13.15.5.3 Runtime Semantics: PropertyDestructuringAssignmentEvaluationに4. If Initializeropt is present and v is undefined, thenとあることから分かる。

ここからNode.jsのfs.readを使ってみていく。

node/lib/fs.js#L605~ に以下のコードがある。

function read(fd, buffer, offsetOrOptions, length, position, callback) {
...
      } else if (arguments.length === 3) {
      // This is fs.read(fd, bufferOrParams, callback)
        if (!isArrayBufferView(buffer)) {
        // This is fs.read(fd, params, callback)
          params = buffer;
          ({ buffer = Buffer.alloc(16384) } = params ?? kEmptyObject);
        }
        ...
...
   ({
      offset = 0,
      length = buffer.byteLength - offset,
      position = null,
    } = params ?? kEmptyObject);
...
   validateBuffer(buffer);
...

例えばfs.read(fd, {offset: 1}, callback)というように呼び出すと、
params = {offset: 1}
({ buffer = Buffer.alloc(16384) } = {offset: 1}
buffer = Buffer.alloc(16384)
というように処理される。

もしfs.read(fd, {buffer: null}, callback)というように呼び出すと、
params = {buffer: null}
({ buffer = Buffer.alloc(16384) } = {buffer: null}
buffer = null
というように処理される。
この場合、validateBufferが呼びされる前にlength = buffer.byteLength - offsetでbuffer = nullが参照されてしまう。

これを解決したのがこのPR。
github.com

ちなみにdeno_stdは現時点でNodeのv18.8.0との互換を保っているため、以下のように意図的にnullを参照するようにしている。
github.com

// @ts-ignore: Intentionally create TypeError for passing test-fs-read.js#L87
length = opt.buffer.byteLength;

2022-08-10

RFC8259 の「Implementations MUST NOT add a byte order mark to the beginning of a networked-transmitted JSON text」について

JavaScript

『プログラマのための文字コード技術入門』の p.216 にある、
「JSONでは、データ先頭にBOMをつけないことが求められています(RFC8259)」
についてちょっとだけ詳しく調べる。

[改訂新版]プログラマのための文字コード技術入門 (WEB+DB PRESS plusシリーズ)

何が変わったのか

実装

候補の列挙

typoキーの検出

typoキーの判定

plugin.typo.fooの検出

候補の判定

結果の表示

展望

新しい仕様を追加する

既存の仕様を変更する

仕様の変更の後継をする

開発のプロセスを改善する

開発の方針を決める