CDKコントリビュートことはじめ

はじめに

年末に息子が産まれ、4ヶ月の育休に突入しました。

運よく寝ている間のフリータイムがそこそこ生まれたので、大変お世話になっているOSSのAWS CDKにコントリビュートしてみようと思い立ちました。

結果として大いにハマり、現在Star Contributorまでなれています。

ただし、最初の1PRのハードルが高かったので、同様の境遇の方に向けて具体的な手順などお伝えできればと思い、記事にしてみました。

こちらの記事(大変お世話になりました)などのN番煎じ感が否めませんが、 統合テスト周りの情報があまりなかったので、そのあたりを重点的に補完できればと思っています。

speakerdeck.com

基本的にはcontributiong guideを参考にしてください。 (といいつつ、これが分かりづらく辛かったのが記事作成のモチベーションです) github.com

前提知識

L1はCloudformationから自動生成されるため、我々の方でいじることはありません。 CDKでの実装内容としてはL2上でのL1操作が基本になります。

L2の新規作成も出来ますが、RFCに則る必要があるなどハードルが高いので割愛します。 (私もやったことはありません)

github.com

手順

環境構築

開発環境を構築します。私はlocalで実施しますが、gitpodでやれたりもします。お好みで。

まずはclone & install

git clone https://github.com/{your-account}/aws-cdk.git
cd aws-cdk
yarn install

その後、aws-cdk-libをビルドします。結構時間かかるので気長に待ちましょう。

npx lerna run build --scope=aws-cdk-lib

issue探索

まずは取り組む課題の明確化です。どうにか自分で対応出来そうなissueを見つけてみます。

effort/small or effort/medium ラベルでフィルタしつつ、自分の詳しいサービスに関するものを眺めて見るのが良いと思います。 ラベリングは割と適当なので、smallでも難しかったりmediumでも簡単だったりします。

取り組みやすいパターン

  1. エンジンやインスタンスのバージョン追加

enumに新バージョンを追加するだけです。一番カンタン。後述する統合テストも多分不要です。

github.com

  1. Cloudformation(L1)で渡せるPropertyをL2の引数に追加

L1はCloudformationから自動生成されるため全てのpropsに対応していますが、L2は都度都度手動で追加していく必要があるため、結構多くのpropsが未対応だったりします。特に、新規でL1に追加されたものが狙い目です。

github.com

  1. deploy時にエラーとなるパラメータをsynth時にチェック

Cloudformationのdocumentに許容されるpropsの条件が明記されています。これを満たしていないとdeploy時にエラーとなりますが、synthの段階でthrow ErrorしてあげるとUXが良くなります。 こちらも統合テストを書かなくてもOKなので、取り組みやすさ◎です。

github.com

もちろん、自分で新規issueを発行してもOKです。既存issueに取り組むと「あれっ？ここの実装、修正したほうが良さそう..??」と気づくことが多々あります。そんなときはissue発行して取り組んでみましょう

特にissueへのコメントは不要です。ガンガン進めちゃいましょう。

今回は例として以下のissueを題材に進めてみたいと思います。

github.com

設計

整理

まずはissue内容を整理します。 L1コンストラクトであるCfnTopicにはArchivePolicyという引数がありますが、これはL2コンストラクトであるTopicからは設定できないようです。そこで、新しくこれを設定できるような引数を追加すれば良さそうです。

これはパターン2の「Cloudformation(L1)で渡せるPropertyをL2の引数に追加」ですね。

仕様調査

Cloudformationのドキュメントには、ArchivePolicyについて以下のように記述されていました。

パラメータはJsonを受け取れるとしか書いてありません。。。不親切ですね。(あるある)

Json形式の情報が見当たらないので、マネコンでSNSトピックを作りArchivePolicyを設定してみます。すると、以下のように丁寧にJSON形式まで表示されていました。

これに則り、以下のようにパラメータを渡してあげれば良さそうです。

// L1コンストラクタでの設定例
const topic = new CfnTopic(this, 'Topic', {
      archivePolicy: {
        MessageRetentionPeriod: 10,
      },
}

また、archivePolicyはFIFOトピックのみで有効なようです。 これは後ほどバリデーションします。

L2仕様検討

Cfnに沿って愚直に作ると、以下のようなInterfaceを定義しTopicPropsに追加してあげれば良さそうです。

interface ArchivePolicy {
  MessageRetentionPeriod: number
}

export interface TopicProps {
  ... // 既存の引数群
   archivePolicy?: ArchivePolicy // 新しいL2の引数として追加
}

const resource = new CfnTopic(this, 'Resource', {
    ... // 既存のパラメータ
    archivePolicy: props.archivePolicy, // CfnTopicへ渡すパラメータを追加
    });

しかしこの場合、ユーザは以下のようなオブジェクトを自作してL2を呼び出す必要があり、あまり直感的とは言えません。

// これを自作する
const archivePolicy = {
    messageRetentionPeriod: 10 // day単位
}

const myTopic = sns.Topic(this, 'MyTopic', {
  archivePolicy,
})

そこで、よりユーザフレンドリーとなるように、messageRetentionPeriodInDaysという引数を受け取るようにしてみましょう。

export interface TopicProps {
  ... // 既存の引数群
   messageRetentionPeriodInDays?: 10 // 新しいL2の引数として追加
}

const period = props.messageRetentionPeriodInDays

const resource = new CfnTopic(this, 'Resource', {
    ... // 既存のパラメータ
    // CfnTopicへ渡すパラメータをL2内で作成
    archivePolicy: period !== undefined ? {
        messageRetentionPeriod: period,
    } : undefined 
    });

これにより、ユーザは日数をL2へ引数として渡すだけで設定が完了します。さっきより使い勝手が良さそうですね。

実装

コードを実装していきます。上記に沿って、aws-cdk/packages/aws-cdk-lib/aws-sns/lib/topic.tsをどんどん修正していきます。

バリデーション

ついでに、パラメータのバリデーションも追加します。 今回はmessageRetentionPeriodInDaysが以下の条件を満たすかチェックします。 - FIFOトピックに適用されているか - 1~365の整数

    // FIFOトピックに適用されているか
    if (props.messageRetentionPeriodInDays && !props.fifo) {
      throw new Error('`messageRetentionPeriodInDays` is only valid for FIFO SNS topics.');
    }
    // 1-365の整数か？
    if (
      props.messageRetentionPeriodInDays !== undefined
      && !Token.isUnresolved(props.messageRetentionPeriodInDays)
      && (!Number.isInteger(props.messageRetentionPeriodInDays) || props.messageRetentionPeriodInDays > 365 || props.messageRetentionPeriodInDays < 1)
    ) {
      throw new Error('`messageRetentionPeriodInDays` must be an integer between 1 and 365');
    }

Token

CDKではデプロイ後まで確定しない値(ex. ARN)をTokenという形で扱います。

docs.aws.amazon.com

引数としてstring or numberを受け取るとき、念のためTokenの可能性も考慮します。 といいつつ、Tokenに対して値のチェックは実行できないため、あくまでTokenでないことを確認すればOKです

// before
if (props.someValue > 10) { ... }

// after
import { Token } from '../../core';
if (!Token.isUnresolved(props.someValue) && props.someValue) { ... }

テスト

テストには単体テストと結合テストの2種類が存在します。基本的にはどちらも作成してからPRを提出します。

単体テスト

cdk synthにより生成されるCloudformationテンプレートが望み通りのものとなっているかをチェックします。 aws-cdk/packages/aws-sns/test/topic.test.tsに以下のようなテストを追記していきます。

    test('specify message retention period in days', () => {
      // GIVEN
      const app = new cdk.App();
      const stack = new cdk.Stack(app);

      // WHEN
      new sns.Topic(stack, 'MyTopic', {
        fifo: true,
        messageRetentionPeriodInDays: 10,
      });

      // THEN
      Template.fromStack(stack).hasResourceProperties('AWS::SNS::Topic', {
        ArchivePolicy: {
          MessageRetentionPeriod: 10,
        },
        FifoTopic: true,
      });
    });

直感的でわかりやすいですね！

同様に、バリデーションについてもテストを追加していきます。 少しバリエーションを加えて、test.eachを活用してみます。これは、配列の要素を順番にtestしていくものです。 全ての要素でちゃんとErrorがthrowされればOKです。

    test.each([0, 366, 12.3, NaN])('throw error if message retention period is invalid value "%s"', (days) => {
      // GIVEN
      const app = new cdk.App();
      const stack = new cdk.Stack(app);

      // THEN
      expect(() => new sns.Topic(stack, 'MyTopic', {
        fifo: true,
        messageRetentionPeriodInDays: days,
      })).toThrow(/`messageRetentionPeriodInDays` must be an integer between 1 and 365/);
    });

test実行は以下のコマンドで行えます。

yarn test topic.test.ts

しっかりテスト通過すればOKです！

結合テスト

実際にCDKテンプレートを自アカウントにデプロイできるかをチェックします。 公式の手順はこちら。

github.com

  1. CDKの環境設定 自分のアカウントへCDK deployできるようにします。AWS CLIの設定及びCDK bootstrapを行えばOKです。

  2. 結合テストファイルの作成

packages/@aws-cdk-testing/framework-integ/test/aws-sns/testに新規の結合テストファイルを作成します。 今回はinteg.sns-fifo-archive-policy.tsを作成します。

import { App, Stack, StackProps } from 'aws-cdk-lib';
import { Topic } from 'aws-cdk-lib/aws-sns';
import * as integ from '@aws-cdk/integ-tests-alpha';

// デプロイするスタックの定義
class SNSFifoArchivePolicyStack extends Stack {
  constructor(scope: App, id: string, props?: StackProps) {
    super(scope, id, props);

    // 早速、作成した引数を使ってみます
    new Topic(this, 'MyTopic', {
      fifo: true,
      messageRetentionPeriodInDays: 12,
    });
  }
}

const app = new App();
const stack = new SNSFifoArchivePolicyStack(app, 'SNSFifoArchivePolicyStack');

// おまじない
new integ.IntegTest(app, 'SqsTest', {
  testCases: [stack],
});
  1. @aws-cdk-testing/framework-integ をビルド
npx lerna run build --scope=@aws-cdk-testing/framework-integ
  1. 生成されたJSファイルを使ってyarn integを実行 TSファイルではないので注意してください。 これにより、cdk deploy → スタックデプロイ完了 → snapshotファイル生成 → スタック削除 → 完了 を自動で実行してくれます。
$ yarn integ ./packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo-archive-policy.js --update-on-failed
yarn run v1.22.21
integ-runner --language javascript test/aws-sns/test/integ.sns-fifo-archive-policy.js --update-on-failed

Verifying integration test snapshots...

  NEW        aws-sns/test/integ.sns-fifo-archive-policy 1.95s

Snapshot Results: 

Tests:    1 failed, 1 total
Failed: /Users/kazuhoshinozuka/git/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo-archive-policy.js

Running integration tests for failed tests...

Running in parallel across regions: us-east-1, us-east-2, us-west-2
Running test /Users/kazuhoshinozuka/git/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo-archive-policy.js in us-east-1
  SUCCESS    aws-sns/test/integ.sns-fifo-archive-policy.js/DefaultTest 165.908s
       NO ASSERTIONS

Test Results: 

Tests:    1 passed, 1 total
✨  Done in 168.56s.

これで生成されたpackages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo-archive-policy.js.snapshotフォルダをまるっとgit add し、ブランチに追加します。

README作成

feature実装の際はREADMEの作成が必須です。 既存のREADMEに追記し、ブランチにaddします

PR作成

一般的な流れでPRを作成すればOKです。テンプレが設定されているので、いい感じに埋めればOKです。

レビュー

以上を満たしてPRを作成すると、自動的にpr/needs-community-reviewラベルが付与されます。いつかcommunity reviewerがレビューしてくれますので、レビュー内容に適宜対応していきましょう。 community reviewerがapproveすると、今度は自動的にpr/needs-maintainer-reviewラベルが付与されるので、maintainerのレビューを待ちます。 maintainerにもapproveされれば自動的にマージされます！お疲れ様でした。

概ね1週間くらいはレビュー待ちの期間があります。 簡易な修正なら即日マージされますし、大きなものだと数ヶ月単位でかかると思います。 気長に待ちましょう。

最後に

困ったときはdistinguish contributorの@go-to-kさんがとても優しく質問に対応していただけると思います。(勝手に宣伝) ハードルが高ければ私でも大丈夫です (@nixieminton)

CDKのissueは2000個ほど積まれていますし、issue化されていない問題は星の数ほどあると思います！ ぜひみんなで良いものにしていきましょう！