はじめに
年末に息子が産まれ、4ヶ月の育休に突入しました。
運よく寝ている間のフリータイムがそこそこ生まれたので、大変お世話になっているOSSのAWS CDKにコントリビュートしてみようと思い立ちました。
結果として大いにハマり、現在Star Contributorまでなれています。
ただし、最初の1PRのハードルが高かったので、同様の境遇の方に向けて具体的な手順などお伝えできればと思い、記事にしてみました。
こちらの記事(大変お世話になりました)などのN番煎じ感が否めませんが、 統合テスト周りの情報があまりなかったので、そのあたりを重点的に補完できればと思っています。
基本的にはcontributiong guideを参考にしてください。 (といいつつ、これが分かりづらく辛かったのが記事作成のモチベーションです) github.com
前提知識
L1はCloudformationから自動生成されるため、我々の方でいじることはありません。 CDKでの実装内容としてはL2上でのL1操作が基本になります。
L2の新規作成も出来ますが、RFCに則る必要があるなどハードルが高いので割愛します。 (私もやったことはありません)
手順
環境構築
開発環境を構築します。私は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でも簡単だったりします。
取り組みやすいパターン
- エンジンやインスタンスのバージョン追加
enumに新バージョンを追加するだけです。一番カンタン。後述する統合テストも多分不要です。
- Cloudformation(L1)で渡せるPropertyをL2の引数に追加
L1はCloudformationから自動生成されるため全てのpropsに対応していますが、L2は都度都度手動で追加していく必要があるため、結構多くのpropsが未対応だったりします。特に、新規でL1に追加されたものが狙い目です。
- deploy時にエラーとなるパラメータをsynth時にチェック
Cloudformationのdocumentに許容されるpropsの条件が明記されています。これを満たしていないとdeploy時にエラーとなりますが、synthの段階でthrow ErrorしてあげるとUXが良くなります。 こちらも統合テストを書かなくてもOKなので、取り組みやすさ◎です。
もちろん、自分で新規issueを発行してもOKです。既存issueに取り組むと「あれっ？ここの実装、修正したほうが良さそう..??」と気づくことが多々あります。そんなときはissue発行して取り組んでみましょう
特にissueへのコメントは不要です。ガンガン進めちゃいましょう。
今回は例として以下のissueを題材に進めてみたいと思います。
設計
整理
まずは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という形で扱います。
引数として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テンプレートを自アカウントにデプロイできるかをチェックします。 公式の手順はこちら。
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],
});
- @aws-cdk-testing/framework-integ をビルド
npx lerna run build --scope=@aws-cdk-testing/framework-integ
- 生成された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化されていない問題は星の数ほどあると思います！ ぜひみんなで良いものにしていきましょう！
