脆弱性管理コードレビューおよび開発標準
目的
この標準は、脆弱性管理チームで行われるすべてのエンジニアリングおよび開発作業の要件を文書化します。
スコープ
このポリシーは以下の要件を定めます:
- エンジニアリングまたは開発作業を必要とする Issue や機能の文書化
- コードコントリビューションと IDE 設定に関する一般的なガイドライン
- マイルストーン計画中にマイルストーンに含めるエンジニアリング作業のサイジング、スコーピング、計画
- エンジニアリングまたは開発作業の重み(推定時間)サイズ(重みに基づく)の見積もりと伝達
- 脆弱性管理ツールへのコントリビューション時のチームメンバーのコードレビュー
- マイルストーンに何を含めて作業をスケジュールするかの決定方法
このポリシーは、GitLab の 脆弱性管理標準 を維持するために必要なプロセスを提供、監視、自動化することの一環として、脆弱性管理チームが所有・保守するプロジェクトにのみ適用されます。GitLab プロダクトそのものや関連プロジェクトを含む、他のプロジェクトへのチームのコントリビューションは対象外です。
ロールと責任
| 責任 | ロール |
|---|---|
| 全体的なマイルストーン計画とキャパシティ管理に関する最終決定 | Engineering Manager |
| この標準の維持と、この標準に対するフィードバック・提案のレビュー | Staff Security Engineer / Engineering Manager |
| この標準に従って計画しているエンジニアリング作業の文書化、スコーピング、サイジング、ラベリング | Security Engineer |
| この標準に定められたガイドラインに従ってマージリクエストとしてコードを投稿 | Security Engineer |
| この標準に従って、脆弱性管理プロジェクトへの他のチームメンバーのコントリビューションをレビュー | Security Engineer |
定義
コードレビュー
コードレビューとは、コードを含むマージリクエストがこの標準だけでなく、MR と関連 Issue の宣言された目標を満たしていることを検証するために、あるチームメンバーが別のチームメンバーを支援することが期待されるプロセスとルールです。この標準は、脆弱性管理チームが使用する新規または既存のツールおよびアプリケーションへのマージのために他のチームメンバーの作業をレビューする際に、何をすべきで、何をすべきでないかに適用されます。
開発
開発とは、文書化された結果を達成するために、プログラミング言語と関連する設定を使用して、プログラムやアプリケーションに変更を加えたり、新しく構築したりすることを指します。
エンジニアリング
エンジニアリング作業には、開発作業に加え、インフラ、計画、関連するすべてのドキュメンテーション作業などの関連作業も含まれます。私たちが使用するプロセス、外部システム、ツールの大部分はコードによって管理されているか、またはコードであるため、この標準は主にエンジニアリング作業に適用されます。
優先度
優先度とは、Issue が脆弱性管理チームの全体的な優先事項にどれだけ整合しているかの尺度です。脆弱性管理チームの優先事項は、チーム戦略、運用作業の重要性、顧客や他の GitLab チームへのコミットメント、特定の期間内に提供することにコミットしている結果との全体的な整合性、および作業をどれだけ早急に提供する必要があるかの組み合わせです。
サイジング
Issue の重みと、エンジニアリングの推定工数(バグ修正であれ、チームの優先事項に沿った成果の提供であれ)を定義するために使われる ~vuln-mgmt::small などのサイジングラベルの組み合わせ。
マイルストーン
脆弱性管理での開発の目的では、作業の計画に GitLab リリースマイルストーン を使用します。マイルストーンを計画する際は、マイルストーンの開始日から終了日までを計画します。私たちの目的では、終了日はマイルストーン期間全体の終わりです。GitLab の開発およびリリースサイクルでは、マイルストーン期間の終了後にリリースプロセスが続き、その後 GitLab のリリース日になります。私たちはリリース日をマイルストーン終了日として使用するのではなく、リンクされたリリース開発マイルストーンリストに記載された実際の終了日を使用します。
マイルストーン計画
特定のチームのマイルストーンに対してどの作業を計画するかを決定するプロセス。マイルストーン計画は主に Engineering Manager によって管理されます。マイルストーン計画は通常、チームの優先事項とキャパシティによって駆動されます。
ベロシティ
作業のサイジング時に使用する標準化された単位(例: Issue の重み)に基づき、特定の繰り返し可能な間隔の間に実施されたエンジニアリング作業の尺度。チームのキャパシティを理解するのに関連します。
キャパシティ
チームのキャパシティは、チームのサイズ、責任、不在、その他の要因に基づいて、特定の期間(例: マイルストーン)にどれだけの作業を妥当に提供できると期待されるかの一般的な尺度です。これは通常、ベロシティを観察し、追加のチームメンバーを増やしたり、レトロスペクティブを実施してチームのベロシティに対する障害に対処することで増加します。
抽象化
このポリシーのコンテキストでの抽象化は、既存のコードをより汎用的にし、ロジックの繰り返しを取り除くために追加されたあらゆるコードと見なされます。
抽象化の例:
- 外部システムからのデータのような実装固有のデータを一般化するために、より高レベルのより汎用的なデータ表現をモデリングする
- いくつかの場所で繰り返されている可能性があるロジックについて、わずかな違いを伴う類似のロジックを、より汎用的な実装に置き換える
MVP (Minimum Viable Product)
MVP は、アイデア、機能、または修正に対する最小限の実装を表します。MVP は、目標やアイデアを提供または検証するために最小限の変更や新機能を提供することにスコープされます。MVP のゴールは、エンジニアリング作業の修正や機能に取り組む際にオーバーエンジニアリングを防ぐことです。MVP は、エンジニアリングソリューションでビジネス問題を解決するように設計された作業に主に適用され、文書化されたビジネス問題に対処できる最小限のエンジニアリング工数が MVP と見なされます。MVP を構築・イテレーションし、MVP を使用してアイデアを実証し結果を共有することは、私たちの Iteration、Results for Customers、Collaboration の価値観に沿っています。
MVC (Minimum Viable Change)
MVC(Model View Controller アプリケーションパターンと混同しないこと)。最小実行可能変更とは、特定の定義された問題を解決するためにコードまたはインフラに加えられる最小限の変更です。最小実行可能変更を行うことにフォーカスすることは、結果を達成するためのオーバーエンジニアリングを防ぐ方法であり、私たちの Iteration および Results for Customers の価値観に沿っています。
実験
実験は、既知の目的と固定された目標を持つ、独立した、新しく、しばしば短期間のプロジェクトです。これらは通常、適切な設計と実装にさらなるエンジニアリング工数を投資する前に、アイデアを実証するために単一のチームメンバーによって開発されます。私たちは、新機能や能力をツールや自動化に追加することに時間を費やす前に、それらが期待されるほど有益または効果的かどうかを最初に検証するために実験を行います。
実験は厳密に MVP としてスコープされます。MVP が構築され、アイデアが実証された(または反証された)後は、さらなる作業はエンジニアリング作業として文書化され、この標準に従って計画されるべきです。
実験は最大でも重み 5 までであるべきです。アイデアを実証するためにより多くの時間(つまり 1 週間以上)が必要な場合、それはおそらくより大きな機能や複雑な問題空間であり、計画されスコープされたエンジニアリング作業として扱う必要があります。
実験は Vulnerability Management Internal プロジェクトで開発されるべきです。
実験のためには、通常、ライセンスの考慮事項は検証されるまでアプローチされず、その時点で適切であれば公開される可能性があるため、公開プロジェクトよりも内部プロジェクトが好まれます。
メンテナンス性
これはかなり広い用語ですが、ソフトウェアエンジニアリングのコンテキストでは、コードまたはインフラに対するメンテナンスを行うことの容易さに焦点を当てます。
いくつかの例を挙げます:
- パッケージマネージャーでソフトウェア依存関係を管理し、依存関係の更新とインストールを容易にする。
- インフラを Infrastructure as Code として管理し、インフラの更新と再構築を容易にし、変更のレビューを促進する。
- ドキュメントを持つ。実装の詳細と取り組む Issue を文書化し、最も重要なのはなぜそれらが存在するかを文書化することで、チームメンバーが変更を投稿する前のリサーチと発見にかかる時間を削減します。
- 適切なテストを持つ。コードロジックのテストは、変更の検証をはるかに容易にし、ベロシティを向上させ、コードを投稿しやすくします。
- シンプルな実装。これは依存関係の選択にも適用できます。ツールに対して MVC Web アプリケーションフレームワーク全体を使用する代わりに、シンプルな HTTP ライブラリを使用することができるかもしれません。前者は、メンテナンス性に良いコントリビューションを行うために必要な学習曲線がはるかに少なくなります。
メンテナンス性を損なうものの例:
- チームメンバーがコントリビューションや実行に困難に直面する高度に複雑な環境依存関係を持つテストは、ベロシティを遅らせ、テストの採用を低下させます。
- 依存関係としてのフォークされたライブラリ、およびよく保守されているコミュニティ保守のライブラリが代わりに使用できるカスタム実装。これらは、別のアプローチで回避できるメンテナンスの負担をもたらします。
- ソリューションのオーバーエンジニアリングと正当化されていない複雑さ。MVP の提供にフォーカスすることで、コードのイテレーションを容易にします。
例外
このポリシーにはいくつかの例外があります。これらの例外が適用される場合、私たちはこの標準を最大限に維持するよう最善を尽くすべきです。イノベーションとイテレーションをブロックすることを避けるために、以下のリストには、状況とポリシーのどの部分が適用されないかを記載します:
実験
実験 は コードレビュー の要件から免除されます。新しい技術やツールを検証するための実験を開発する際、おそらく一人で実験に取り組んでいるため、イテレーション中は コードレビュー 標準から免除されます。もちろん、問題についてチームメンバーに助けを求めたり、コードをレビューしてもらったりすることは自由ですが、このポリシーでは要求されていません。
リアクティブな作業
リアクティブな作業 はしばしば予想できず、時には緊急事態を表します。その結果、それらに対処することは私たちの マイルストーン計画 から免除されます。マイルストーンを計画する際は、予想できない緊急またはリアクティブな作業を考慮するためにキャパシティの一部を空けておきます。
ポリシー
計画
マイルストーン計画ミーティング
新しいマイルストーンが始まる前に、毎週のチームシンクコールがマイルストーン計画ミーティングとして使用されます。
マイルストーン計画ミーティングの日付は、チームミーティングと次のマイルストーンの開始日の間に最も少ない米国営業日数があるチームシンクの日付になります。例えば、マイルストーンが水曜日に始まり、チームシンクが木曜日にある場合、マイルストーン開始日の前の木曜日ではなく、マイルストーン開始日の翌週の木曜日がマイルストーン計画ミーティングになります。
マイルストーンを計画する際は、メインの GitLab プロジェクトの GitLab リリースマイルストーン に表示される開始日と終了日を使用します。プロダクト作業を除き、リリース日(マイルストーンの終了)の 1 週間前に機能フリーズを行わないため、マイルストーンを計画する際は、マイルストーンの開始から GitLab リリースマイルストーンの終了日までを計画します。リリース日は終了日に続くものですが、終了日はリリース日と同じではないことに注意してください。これはリンクされたマイルストーンリストに記載されている日付と一致しています。
このミーティングでは、チームメンバーは次のマイルストーンに含める Issue について議論する準備ができていることが期待されます。 このミーティングは、全体的なマイルストーン計画に責任を持つチームメンバーである Engineering Manager によって運営されます。彼らが利用できない場合、通常は Staff Engineer がミーティングを運営しますが、このポリシーの要件ではなく、可用性の理由で別のチームメンバーが代わりに参加するよう求められる場合があります。
ミーティングの前に、メインの脆弱性管理トラッカーで milestone_planning.md Issue テンプレートを使用して新しいマイルストーン計画 Issue を作成する必要があります。これは、計画ミーティングの結果を文書化し、チームメンバーの参考資料として機能します。
ミーティング中の主な目標は以下の通りです:
- Engineering Manager(または代理)に現在のチームの優先事項をチームメンバーに思い出させる
- チームメンバーが順番にマイルストーンに含める Issue について議論する
- このポリシーに基づいて含めるために議論されている Issue をレビューし、何を含めるかについて合意に達する
- 可用性の制約(休暇、病気など)について議論し、適切に割り当てを調整する
- チームメンバーに毎週のリアクティブローテーションと、関連する運用作業を処理するための十分な空きキャパシティを残す(理想的には ~5 ポイント)
- 前のマイルストーンから継続またはフォローアップ作業がある作業の処理方法を決定する
マイルストーンに含めるための Issue 要件
Issue がマイルストーンに含まれるためには、以下を満たす必要があります:
- 適切に見積もられている(サイジングとスコアリング)
- このページに記載されているエンジニアリング Issue ポリシーに従って適切に文書化されている
- 理解可能な提案されたアプローチと、理想的には実装が文書化されている
- マイルストーン中に完了できる
- 現在のチーム、部門、ディビジョン、会社の目標と優先事項に整合している
結果
マイルストーンが計画された後、チームの各エンジニアはマイルストーン中に取り組む計画された作業について明確なビューを持つ必要があります。すべての適切な Issue は、関連する現在のマイルストーンに設定されます。すべての Issue は割り当てられ、適切なマイルストーンが設定されている必要があります。マイルストーンは、マイルストーンで実施されるすべての計画された作業を表す必要があります。
計画の結果は、計画ミーティングの前に作成されたマイルストーン計画 Issue に反映され、計画された作業を Issue に含めるべきです。この Issue は、認識のためにチームと Product Security リーダーシップと共有する必要があります。
エンジニアリング Issue
脆弱性管理チームで Issue をどのように文書化し、取り組むかは、このポリシーに従う必要があります。これは高レベルのガイダンスですが、Issue 品質に対する期待を詳細に説明するポリシーでもあります。この標準を満たさない Issue は、通常、マイルストーンへの含めることが考慮されないため、このページの例外セクションに記載されているとおり、取り組まないでください。
エンジニアリング Issue テンプレート
Vulnerability Management チームのメイントラッカー と VulnMapper プロジェクトの Issue トラッカーには Engineering Work という名前の Issue テンプレートがあります。これらのポリシーが進化するにつれて、このテンプレートをイテレーションすることを目指しており、このテンプレートが私たちが保守する他のプロジェクトにある場合は、適切に追加する必要があります。これは、エンジニアリング作業の計画、文書化、正当化に必要な情報を覚えやすくするためのシンプルなフォーマットを設定します。新しいエンジニアリング Issue を作成するときに使用する必要があり、マイルストーン計画の前に既存の Issue を更新するときのガイドとして使用すべきです。
Issue のサイジング/スコアリング
計画の一環として、すべての Issue を適切なラベルと重みでサイジングおよびスコアリングする必要があります。これらのラベルと重み付け方法論は、脆弱性管理チームページの「Planning」 と Development Labels ページに詳しく文書化されています。
優先度
Issue は、Engineering Manager によって実施される定期的な優先度の調整とレビュー時に優先度ラベルが付けられます。Issue に優先度を追加する具体的なガイダンスがない限り、または実証されたビジネスへの影響がマイルストーン計画に含まれていない作業を駆動する場合を除き、脆弱性管理チームメンバーが追加すべきではありません。
マイルストーン内、またはマイルストーン計画セッション外でマイルストーンに含めようとしている Issue のみが優先度ラベルを持つべきです。優先度ラベルは、最も顧客への影響が大きいフォーカスエリアに関連する作業をより良く反映するために、ビジネスやチームのコミットメントや優先事項の変化に基づいてマイルストーン中に変更される可能性があります。これらの変更は、毎週のレビューの結果として、または優先度の変更を駆動する新しい情報の結果として行われます。
すべての場合において、マイルストーン中の優先度の変更には、優先度の変更の背景を伝え、認識してもらうために、割り当てられたエンジニアにタグ付けしてコメントも含める必要があります。これにより、特定のマイルストーンの作業の優先順位付けを調整できます。
Engineering Division Priority Triage ハンドブックページ と 脆弱性管理開発ラベルハンドブックページ に記載されている標準の優先度ラベルを使用しています。
why の文書化
Issue は why(なぜ)を説明する必要があります。既存のコードに機能を追加することを記述する Issue では、なぜその機能が必要かを文書化する必要があります。既存のコードを変更する Issue では、なぜその変更が必要または望ましいかを文書化する必要があります。メンテナンス Issue でも、why を詳細に記述する必要がありますが、通常はずっと短い説明になります。
例: プロジェクトの依存関係を更新する Issue は、「Why: 顧客とチームに対してモデルとして示したい良いプラクティスを実証することで、安全なソフトウェア配信ライフサイクルを維持するため」と言うかもしれません。
例: バグ修正を記述する Issue は、「Why: このツールのバグによって他のチームから観察された問題を記述するこの Issue にリンクされた Issue が引き起こされています。チーム/全チームメンバー/顧客に約束された機能を提供するために、このバグを修正する必要があります。このプロセスはコンプライアンス操作の重要な部分であり、これを修正しないとコンプライアンス要件に影響します。」と言うかもしれません。
例: 新機能を記述する Issue は、「Why: トリアージ Issue にこの追加情報を含めることで、毎週 10 時間の手動トリアージ作業を節約できると他のチームからフィードバックがありました。実装がかなり容易であることを考えると、機能を追加することで全体的な会社の効率が向上し、セキュリティ問題に対処しやすくなります。」と言うかもしれません。
問題またはゴールの文書化
これは why の拡張です。why は何らかの作業の高レベルの結果と推進力を文書化する一方、GitLab と他者への私たちの責任のコンテキストにおける問題またはゴールは、エンジニアリング作業を提案しているツール、自動化、またはアプリケーションのコンテキストで追加または変更する必要がある変更、修正、または新機能のより詳細な説明です。
Issue の why がチームメンバーに何を提供するかにフォーカスする一方、問題またはゴールは、希望する結果を実現するために脆弱性管理が保守する特定のツールやインフラのどのデータソース、機能、能力を追加または変更する必要があるかにフォーカスします。
例: 新機能を提供する際、Issue の why は、他のチームメンバーが要求した新しいデータの一部を記述するかもしれません。この状況で、問題またはゴールは、現在の実装の制限と、必要な情報を提供、保存、または表現できない理由、および何が提供される必要があるかを記述します。例えば、「現在のプロバイダー実装では、関連する Linux ディストリビューションのベンダー CVSS データにアクセスできません。要求された機能を提供するには、既存のアドバイザリ処理コードの一部として、Alpine と Chainguard のアドバイザリデータを処理するサポートを追加する必要があります。」のようなものです。
問題またはゴールの良い説明は、ビジネス問題を参照し、現在の技術的制限を文書化し、必要な結果を生み出す MVC を提案するべきです。
高レベルの計画の文書化
エンジニアリング Issue でカバーされる作業を高レベルで文書化する必要があります。これは本質的に非常に軽量なソリューション提案と設計演習であり、Issue を文書化する際に、現在の制限、解決すべき問題、Issue のゴールを理解するために作業し、宣言された結果を達成するために実施する必要のある高レベルの技術的変更を文書化できます。
これは問題を定義することよりも、問題を Issue のスコープになる一連の MVC に分解することについてです。他のプロジェクトで必要な作業や、必要なインフラ変更を考慮し、それらの Issue が高レベル計画の一部として依存関係としてリンクされていることを確認するために注意を払う必要があります。
エンジニアリング Issue を読むときに、コードのどの部分が変更されるか、それらのコンポーネントへの意図された変更は何か、必要な高レベルのステップは正確に何かが明確である必要があります。
これは、エンジニアリング作業の文書化、サイジングの検証、提案された実装を理解し検討したことの確認、スコープクリープの削減、必要に応じて他のエンジニアと協働して Issue を引き継ぐことを容易にするのに役立ちます。
Issue の作業
Issue がマイルストーンに含まれ、チームメンバーに割り当てられたら、そのチームメンバーは Issue がマイルストーン中に取り組まれ、進捗状況が最新に保たれるようにする必要があります。
現在、Issue の完了状況を反映するために Issue ステータスを使用していますが、計画中、計画済み、現在作業中、ブロック中、完了などの Issue を識別するのに役立つワークフロー進捗ラベルを Issue に導入することを計画しています。現在、チームメンバーは説明が現在のステータスを反映していることを確認し、実装後に Issue がクローズされるようにする必要があります。
実装中にスコープが変更される場合、例えば Issue のゴールを満たすために追加作業が必要であることが明らかになった場合、追加作業は新しいエンジニアリング Issue に文書化し、現在進行中の Issue にリンクする必要があります。次のマイルストーンの計画に含め、現在の Issue で宣言されたゴールを提供するために追加作業が必要であることに注意する必要があります。
外部要因によりマイルストーン中に優先度や時間関連の要因が変化する場合、現在取り組んでいる既存の Issue を更新する必要があります。これらの要因により Issue に取り組む必要があり、マイルストーン計画に含まれていなかった場合、決定が下せるように、チームの Engineering Manager と Staff Engineer に Issue で通知する必要があります。適切な場合、マイルストーン計画は計画からの逸脱を記録するために更新され、新しい作業を含めます。判断を使用し、操作や他のチームの操作に影響を与えない場合のみ、Engineering Manager またはチームの Staff Engineer の +1 を介してマイルストーン計画への Issue を含めるかどうかの決定を待ち、このハンドブックページに記載されているポリシー例外に注意してください。
コードまたは設定の記述
このポリシーは、脆弱性管理によって保守されるコードを記述または変更する際に従う必要のあるプラクティスを定めます。このポリシーは意図的に要件が非常に少なく、個々のチームメンバーの生産性とコントリビューションへの障壁および非効率の除去にフォーカスしています。
このポリシーは、脆弱性管理によって保守されるプロジェクトへのコードの投稿にのみ適用されます。脆弱性管理外のプロジェクトには、必要な場合に従うべき独自の標準があります。
言語の選択
脆弱性管理チームのメンバーが Go に精通しており、再利用できる優れたライブラリとモジュールのエコシステムがあり、引き出すか拡張できる多くの既存コードがあり、ツールが非常に意見の強いものであり(バッテリーが含まれている)、特定のツールの決定をあまり多く行う必要がないため、私たちは Go でコードを書くことに対して強い好みを持っています。さらに、Go は GitLab CI/CD およびセキュリティスキャン機能で十分にサポートされているため、開発中に GitLab プロダクトを効果的に使用できます。
他のチームとのコラボレーションのために理にかなっている場合、Ruby により精通しているチームのコード投稿やアイデアの実証を支援するために Ruby などの異なる言語を選択することがあります。私たちが構築するすべての Web ベースのアプリケーションについては、Web フロントエンドが必要になり次第、Ruby on Rails も使用する必要があります。Go でのシンプルなステータスページや API を超える Web 開発は、高レベルフレームワークの不足のために非効率的で煩わしいです。Web 向け機能を Rails でコーディングすることは、私たちのコードのパラダイムが Go で書かれた場合よりも GitLab の Rails コードベースに容易に翻訳できるため、GitLab プロダクトへの貢献にも役立ちます。
Python または Bash スクリプトでの自動化の記述は可能な限り避けるべきです。Node.js は私たちが保守する 1 つのプロジェクトで使用されていますが、この言語で堅牢なツールを書く難しさと、エコシステムへの一般的なチームの不慣れのため、Node.js で新しいツールを書いてはいけません。
コードスタイルとリンティング
プロジェクトの CI 設定に適切なコードリンティングを含める必要があります。使用されるルールはカスタムであるべきではなく、カスタマイズが正当化でき、チームの効率またはメンテナンス性に対して実証可能な利益を伴うエンジニアリング作業として計画できる場合を除きます。
プロジェクトでリンターを有効にする際、プロジェクトで使用される主要なプログラミング言語(または複数の言語)のコミュニティで良いプラクティスと見なされる最も一般的なリンターを有効にする必要があります。
良い例:
- Go:
golangci-lint、gofmt、go vetを有効にする必要があります。基本リンティングルールを検証する必要があります。Go は意見の強い言語であり、標準のリンティングルールはかなり一貫しています。 - Python: PEP8 を尊重するリンティングを有効にする必要があります。
pylinとblackの組み合わせが良い選択です。 - Ruby:
rubocopが良い選択です。 - Bash:
shellcheckが良い選択であり、特に一般的なミスをキャッチするのに適しています。
セキュリティスキャン
私たちのプロジェクトには適切なセキュリティスキャンが有効になっている必要があります。
- Infrastructure as Code(Terraform など)の場合、IaC Scanning を有効にする必要があります。
- Go、Ruby、および一般的なソースコードプロジェクトの場合、SAST Scanning を有効にする必要があります。
- すべてのプロジェクトについて、Dependency Scanning と、Secret Push Protection を伴う Secret Detection を有効にする必要があります。
- コンテナをビルドするプロジェクトの場合、Container Scanning を有効にする必要があります。
IDE 設定
チームメンバーは IDE 設定がこの標準のガイドラインと一致し、適切な言語の標準ルールセットを使用するプロジェクトに対して構成したリンティングルールとも一致していることを確認する必要があります。
設定が誤っている IDE は、関連性のないフォーマットとインポートの順序の変更を MR に自動的に導入し、レビューが困難になり、同じコードに協力するチームメンバー間でフォーマットの混乱を引き起こし、それらの変更を削除するために MR を一貫して作り直す必要が生じる場合があります。
IDE の特定の自動フォーマット機能を使用または提案したい場合、まずこの標準と関連プロジェクトのリンティング設定の変更を提案し、導入したいリンティングルールの価値を実証し、他のチームメンバーが IDE で構成すべきカスタムリンティングルールとして文書化する必要があります。カスタムルールがマージされて文書化されたら、IDE をそのフォーマットを自動的に適用するように構成できます。
依存関係管理
依存関係を最新に保つために自動化を有効にするか、保守されているプロジェクトの定期的な依存関係更新の Issue を作成する必要があります。Renovate のようなツールは、依存関係更新の自動化と効率を大幅に向上させ、新しい依存関係がリリースされてから、私たちが保守するコードに含めるための MR が作成されるまでの時間を短縮できます。
現段階では、私たちのプロジェクトで Renovate を使用するための承認された方法はまだなく、脆弱性管理プロジェクトの Renovate を探索および設定するためのオープンな(非公開の)エンジニアリング Issue(vulnmapper、vm_infra)があります。それが利用可能になると、すべてのプロジェクトで Renovate の使用が必須となります。
コードレビュー
マージリクエストが承認されてマージされるためには、以下の条件を満たす必要があります。ポリシーの各部分にはヘッダーがあり、例が含まれています。脆弱性管理チームでコードを書いたりレビューしたりする人は、このポリシーに精通している必要があります。他のチームのチームメンバーには、これは脆弱性管理のメンテナーがあなたの提出物をどのようにレビューするかの参考として提供されます。
自動テストはパスする必要がある
すべての自動テストとコード衛生テストはパスする必要があります。例外がある場合は MR で記録され、MR レビューの一部としてレビューされる必要があります。
例えば、これに対する例外は、失敗しているユニットテストを修正するための MR である可能性があり、その場合、MR で対処されている失敗テストは、変更がマージされてデプロイされるまでパスしない可能性があります。
マージ前にマージリクエストはレビューされる必要がある
実験や、脆弱性管理ツール、インフラ、または運用全般に対するインタラクションや影響がないプロジェクトに取り組んでいる場合を除き、いかなるコントリビューションも、少なくとも 1 人の他の脆弱性管理チームメンバーによってレビューされる必要があります。
例: レビューを求める必要がない時とは、GitLab データに触れず、GitLab チームメンバーや運用にいかなる方法でも干渉しない、まったく新しいセキュリティスキャニングツールなど、個人的な実験に取り組んでいるときです。ただし、このツールがチームや GitLab の他のチームによる一般的な使用に入る前に、このポリシーに沿ったコードレビューが行われる必要があります。検証されると、脆弱性管理プロジェクトに昇格され、このポリシーに準拠する必要があります。
例: 現在サポートしていない新しいタイプのアーティファクトをスキャンできるかを試すツールを作成しており、このツールが画像やパッケージに対して操作を行うが、変更を加えたり公開したりせず、さらなるエンジニアリング作業の前に概念を検証するだけの場合。
別のチームから直接影響を受け、入力が必要な変更の場合、そのチームのメンバーから MR で直接フィードバックを求めるべきです。これの例は、別のチームが責任を持つ Issue や脆弱性に対して操作するように VulnMapper を設定する VulnMapper 機能を有効にすることです。これは、コラボレーションを支援し、影響を受けるチームとのコミュニケーションを私たちの一般的な認識とプロセスに組み込むために設計されています。
レビューは望ましい結果にフォーカスする必要がある
マイルストーンに含まれる作業の文書化とサイジングの一環として、Issue がアプローチを文書化し、必要な工数を見積もり、変更や新機能が必要な理由を文書化することを要求します。変更または新機能が必要な理由は、マージリクエストを承認してマージすることの予想される結果に対してレビューする必要があります。
例: 現在のマイルストーンに含まれる Issue が、外部データソースから導入する特定の新しいフィールドを記述している場合、フィールドが変更によって導入され、元の Issue のコンテキスト内で有用で意味のあるデータが含まれていることを、できる限り(適切な場合はテストで)検証する必要があります。そうであれば、それはマージリクエストを承認する良い理由となります。そうでない場合、それは変更を要求し、承認を保留する妥当な根拠となります。
レビューはメンテナンス性にフォーカスする必要がある
レビュアーとして、コントリビューションがどれだけメンテナンス可能であるかを常に考慮する必要があります。コントリビューションをレビューし、変更の目標や優先度によって正当化されない追加のメンテナンスの負担(依存関係、抽象化、機能、将来の変更をテストできないことなど)が追加されるかどうかを考慮する必要があります。実証可能によりメンテナンス可能な実行可能な代替案を提供できる限り、承認を保留することは適切です。これは、より良く保守されている代替ライブラリ、実装の複雑さの削減、冗長なコードや抽象化の削除、レビューされている変更が意図したとおりに機能することを検証するのに役立つテストの追加の形式を取ることがあります。ただし、これらの提案はこの標準の残りの部分と一致している必要があり、メンテナンス性の懸念を検証する一環として、メンテナンス性の改善を実証する必要があります。
レビューはセキュリティにフォーカスする必要がある
私たちは GitLab Security の一部であり、特に他人のコードやシステムの脆弱性を管理しています。私たち自身のコードにセキュリティ脆弱性を導入することは皮肉で残念なことなので、それを避けるためにできる限りのことをすべきです。すべてのプロジェクトで適切な GitLab セキュリティスキャン機能を構成しますが、提案された変更のセキュリティについて実証可能な懸念がある場合、レビューフィードバックとして提供する必要があります。MR がスキャンに基づいて追加の脆弱性を導入する場合、または手動レビューで実装に脆弱性を見つける場合、承認を保留することは適切です。MR のセキュリティレビューの一環として MR セキュリティウィジェット を使用して MR で導入された脆弱性をレビューする必要があり、すべての新しい所見は解決される必要があります。誤検知の所見は、Application Security Testing の下の適切なチームへの Issue を介してフィードバックとして提供する必要があります。
レビューは個人的な好みにフォーカスしてはいけない
コードをレビューする際、レビュー対象のコードのゴール、機能、適切なパフォーマンスレベルについて客観的でなければなりません。私たちのコードレビューガイドラインと一致する具体的な理由なしに自分なら異なるように実装したと感じる場合、コードを自分の好みに基づいて書き直すことを規定して MR の承認を保留してはいけません。もちろん、自分の好みの理由を実証し、他のチームメンバーと議論することは奨励されますが、これに基づいて作業をブロックしてはいけません。
個人的な好みは、フォーマットの好みや、小さな慣用的な好みなど、コーディングの多くの分野に適用されることがあります。これは「nitpick(あら探し)」の形式を取ることもあり、これはほとんど役に立たないフィードバックであり、コードレビューでは避けるべきです。この種のフィードバックを深く共感をもって議論する 素晴らしい投稿 があります。
レビューはスタイルのためのスタイルにフォーカスしてはいけない
私たちは合理的なスタイル期待を設定し、コード全体にわたって基本的で一貫性のあるリンティングを含めることを目指し、チームメンバーに作業するプロジェクトに対して IDE を適切に構成して、コラボレーションを容易にし、コードを一貫して読みやすくすることを期待しています。自動リンティングを通過した他のチームメンバーのコードのスタイルをレビューすべきではなく、特に個人的な好みに基づいてレビューすべきではありません。コードが宣言された目標を満たし、定義された使用法に基づいて適切に機能的でパフォーマンス的であることが期待される場合、スタイルのフィードバックのために承認を保留すべきではありません。
チームのメンバーがコードをスタイルする方法を変更したい場合、レビューする MR にアドホックな変更を求めるのではなく、希望するスタイル変更に対応するために自動リンティングを変更するエンジニアリング作業を提案するべきです。これはより繰り返し可能で一貫性があり、スタイル変更の必要性と利点をよりよく考慮できます。
MR は無関係な変更を含んではいけない
IDE が作業しているファイル全体を自動的に再フォーマットする場合、またはプロジェクトの作業時にプロジェクトのすべての依存関係をバンプする場合、これらは計画されスコープされた変更をプロジェクトに加えている MR に含まれてはいけません。レビューを保留し、計画されたメンテナンス作業の一部として不要な変更を別の MR に含めるよう求めることは適切です。
コードを早すぎる最適化してはいけない
早すぎる最適化とは、ユースケースに関係なく、コードのゴールを満たすためのリソース制約やスケーラビリティ要件の欠如を無視して、すべてのコードを可能な限りパフォーマンス的にするプロセスを指します。
ただし、変更のゴールに基づいてシステムの制約内で動作するために必要な場合は、絶対にコードを最適化する必要があります。
例: アドバイザリ CVSS、既知のエクスプロイテーション、ベンダースコアリング情報などのいくつかの要因に基づいて GitLab Issue の優先度を計算する必要があります。マージリクエストは、これらの値で呼び出して優先度を浮動小数点数として返すことができるシンプルなヘルパー関数を導入します。関数のロジックの潜在的な最適化に基づいてマージリクエストの承認を保留してはいけません。これは、計画された作業のゴールを満たしているため、正当化される場合を除きます。関数が 100 件のレコードに対して 1 日 1 回呼び出される場合、承認を保留することは不適切です。関数が 1 分ごとに 100,000 回呼び出され、この変更が処理に大きな負荷と遅延を導入することが予想される場合、最適化の提案を行い、承認を保留することは絶対に適切です。
コードは不要な抽象化を導入してはいけない
コードをより理解しやすくし、コードベース内のロジックの繰り返しを削除することは、全体的なコード行数を削減し、コードのメンテナンスとイテレーションをより効率的にする優れた方法です。ただし、既存または新規コードの抽象化は、正当化できる場合にのみ追加されるべきです。不要な抽象化はコードを理解しにくくし、MR の宣言されたゴールの解決に必要のない追加のエンジニアリング工数につながり、レビュアーに余分な作業をもたらす可能性があります。理想的には、抽象化は必要に応じて個別のエンジニアリング Issue として追加されるべきであり、それらを追加することは現在または次のマイルストーンで計画された作業の依存関係として扱われるべきです。
抽象化を追加する正当化を提供することで、レビュアーは、現在レビューされている作業や、このまたは次のマイルストーンに含めるためにも計画され承認された他の作業が抽象化を必要とする理由について必要なコンテキストを持ちます。これはまた、抽象化の必要性、抽象化のレベル、およびツールやアプリケーションのチームのゴールを満たすために必要な詳細を考えていることを確認するのに役立ちます。
例: 現在のマイルストーンに含まれる Issue が、以前は脆弱性検出のみを処理していた既存ツールにサーバーに関する情報を追加するなど、ツールの 1 つに新しいタイプの情報を追加する必要がある場合、MR で導入されたコードによって最小実行可能な情報量が正しくモデリングされていることを検証し、さらなる抽象化が適切であることを検証する必要があります。この例の MR がサーバーを導入し、具体的な正当化なしにクラウド資産のさらなる抽象化も一般的に導入する場合、それはレビュアーが不要な抽象化を削除することを求め、承認を保留する根拠となります。ただし、抽象化の必要性が説明され、文書化され、他の計画された作業によって必要とされている場合、追加の抽象化があっても変更を承認する良い理由となります。
Rule of Three(3 の法則)
一般的なガイドラインとして、3 の法則に従うべきです。同じコードが 3 回現れる場合、メンテナンス性を向上させるために、適切なテストとドキュメントを伴う独立した関数、メソッド、または同様のものにリファクタリングされるべきです。
bfd74782)