Clean up handling of CODEOWNERS updates.

pantheon-systems · Aug 26, 2024 · e740359 · e740359
1 parent 80bc714
commit e740359
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 22 deletions.
diff --git a/src/Cli/OrgCommands.php b/src/Cli/OrgCommands.php
@@ -71,7 +71,7 @@ class OrgCommands extends \Robo\Tasks implements ConfigAwareInterface, LoggerAwa
      *   owners_src: Owners Source
      *   ownerTeam: Owning Team
      *   support_level: Support Level
-     * @default-fields full_name,codeowners,owners_src,circle_vars,circle_contexts,support_level
+     * @default-fields full_name,codeowners,owners_src,support_level,default_branch
      * @default-string-field full_name
      *
      * @return Consolidation\OutputFormatters\StructuredData\RowsOfFields
@@ -281,10 +281,10 @@ public function orgUpdateProjectsInfo($csv_file, $options = [
                 $prBody = $options['pr-body'];
                 // Get column indexes if header row.
                 if ($row_id == 0) {
-                    $projectFullNameIndex = $this->getColumnNumber('full_name', $row);
-                    $projectOrgIndex = $this->getColumnNumber('owner/login', $row);
+                    $projectFullNameIndex = $this->getColumnNumber('Name', $row);
+                    $codeOwnersIndex = $this->getColumnNumber('Code Owners', $row);
                     $projectSupportLevelIndex = $this->getColumnNumber('Support Level', $row);
-                    $projectDefaultBranchIndex = $this->getColumnNumber('default_branch', $row);
+                    $projectDefaultBranchIndex = $this->getColumnNumber('Default Branch', $row);
                     continue;
                 }
                 if (empty($row[0])) {
@@ -294,12 +294,11 @@ public function orgUpdateProjectsInfo($csv_file, $options = [
                 $projectUpdateSupportLevel = $updateSupportLevelBadge;
                 $projectUpdateCodeowners = $updateCodeowners;
                 $projectFullName = $row[$projectFullNameIndex];
-                $projectOrg = $row[$projectOrgIndex];
                 $projectSupportLevel = $row[$projectSupportLevelIndex];
                 $projectDefaultBranch = $row[$projectDefaultBranchIndex];
-                $codeowners = '';
+                $codeowners = $row[$codeOwnersIndex];
                 $ownerSource = '';
-                if ($this->validateProjectFullName($projectFullName) && !empty($projectDefaultBranch) && !empty($projectOrg)) {
+                if ($this->validateProjectFullName($projectFullName) && !empty($projectDefaultBranch)) {
                     // If empty or invalid support level, we won't update it here.
                     if ($projectUpdateSupportLevel && (empty($projectSupportLevel) || !$this->validateProjectSupportLevel($projectSupportLevel))) {
                         $projectUpdateSupportLevel = false;
@@ -311,16 +310,20 @@ public function orgUpdateProjectsInfo($csv_file, $options = [
                         $prBody = $options['pr-body-unsupported'] ?? $prBody;
                     }
                     if ($projectUpdateCodeowners) {
-                        list($codeowners, $ownerSource) = $this->guessCodeowners($api, $projectOrg, $projectFullName);
-                        if (empty($codeowners) || $ownerSource === 'file') {
-                            $projectUpdateCodeowners = false;
-                        } else {
-                            if ($codeownersOnlyApi && $ownerSource !== 'api') {
-                                $projectUpdateCodeowners = false;
-                            } elseif ($codeownersOnlyGuess && $ownerSource !== 'guess') {
+                        // If the csv file does not list a code owner to use, infer from api / guess
+                        if (empty($codeowners)) {
+                            list($projectShortName, $projectOrg) = $this->projectNameAndOrgFromFullName($projectFullName);
+                            list($codeowners, $ownerSource) = $this->guessCodeowners($api, $projectOrg, $projectFullName);
+                            if (empty($codeowners) || $ownerSource === 'file') {
                                 $projectUpdateCodeowners = false;
                             } else {
-                                $codeowners = implode('\n', $codeowners);
+                                if ($codeownersOnlyApi && $ownerSource !== 'api') {
+                                    $projectUpdateCodeowners = false;
+                                } elseif ($codeownersOnlyGuess && $ownerSource !== 'guess') {
+                                    $projectUpdateCodeowners = false;
+                                } else {
+                                    $codeowners = implode('\n', $codeowners);
+                                }
                             }
                         }
                         if ($options['codeowners-only-owner'] && $codeowners !== $options['codeowners-only-owner']) {
@@ -424,22 +427,31 @@ public function orgUpdateProjectsMergePrs($user, $options = [
     }
 
     /**
-     * Validate project full name. Throw exception if invalid.
+     * Validate project full name.
      */
     protected function validateProjectFullName($projectFullName)
+    {
+        $parts = $this->projectNameAndOrgFromFullName($projectFullName);
+        return !empty($parts);
+    }
+
+    /**
+     * Separate project name and org from a full name ("project-name/org").
+     */
+    protected function projectNameAndOrgFromFullName($projectFullName)
     {
         if (empty($projectFullName)) {
-            return false;
+            return [];
         }
         $parts = explode('/', $projectFullName);
         if (count($parts) != 2) {
-            return false;
+            return [];
         }
-        return true;
+        return $parts;
     }
 
     /**
-     * Validate project support level. Throw exception if invalid.
+     * Validate project support level.
      */
     protected function validateProjectSupportLevel($projectSupportLevel)
     {

diff --git a/src/Util/ProjectUpdate.php b/src/Util/ProjectUpdate.php
@@ -63,14 +63,14 @@ public function updateProjectInfo($api, $project, $baseBranch, $branchName, $com
         $readme_changed = false;
 
         if (!empty($codeowners)) {
-            // Append given CODEOWNERS line.
+            // Replace CODEOWNERS with specified new value
             $string_to_add = '* ' . $codeowners . "\n";
             $codeowners_content = '';
             if (file_exists("$dir/CODEOWNERS")) {
                 $codeowners_content = file_get_contents("$dir/CODEOWNERS");
             }
             if (strpos($codeowners_content, $string_to_add) === false) {
-                file_put_contents("$dir/CODEOWNERS", $string_to_add, FILE_APPEND);
+                file_put_contents("$dir/CODEOWNERS", $string_to_add);
                 $workingCopy->add("$dir/CODEOWNERS");
                 $codeowners_changed = true;
             }
@@ -110,6 +110,14 @@ public function updateProjectInfo($api, $project, $baseBranch, $branchName, $com
             $workingCopy->commit($commitMessage);
             $workingCopy->push('fork', $branchName);
             if (!$existingPrFound) {
+                // TODO: We cannot do this too quickly, or we will get intermittent
+                // failures (thrown exceptions):
+                // Validation Failed: Field "head" is invalid, for resource "PullRequest"
+                // Sleeping for five seconds clears up almost all of these occurances.
+                // Maybe we can retry, or better yet, figure out how to detect if
+                // GitHub has finished processing the push and is ready for the PR to
+                // be created.
+                sleep(5);
                 $workingCopy->pr($prTitle, $prBody, $baseBranch, $branchName);
             }
         }