GCSS-1135: Add support for GitHub environment with deployment policy #28

ljuboops257 · 2025-11-18T20:22:03Z

Add GitHub Environment Support with Improved Deployment Policy Structure

This PR adds support for GitHub repository environments and introduces a cleaner, more intuitive deployment policy structure throughout the codebase.

Key Changes

🚀 New Feature: GitHub Environments

Terraform Module: Added support for managing GitHub environments with deployment policies, reviewers, and protection rules
Importer Tool: Can now import existing GitHub environment configurations when enabled
Deployment Policies: Three supported modes:
- protected_branches - Only protected branches can deploy
- selected_branches_and_tags - Specific branch/tag patterns can deploy
- No policy - Any branch can deploy

🎛️ Feature Flag System

New Configuration: import-config.yaml now supports feature flags
Enable with: feature_github_environment: true
Default: All features disabled (backward compatible)
Architecture: Config-driven, no CLI changes needed

  # gcss-config-repo/config/import-config.yaml
  feature_github_environment: true  # Enable environment import

📝 Environment Configuration Structure

  environments:
    - environment: production
      wait_timer: 300
      can_admins_bypass: false
      prevent_self_review: true
      reviewers:
        users: ["octocat"]
        teams: ["platform-team"]  # ⚠️ Teams must have repo access
      deployment_policy:
        policy_type: protected_branches

    - environment: staging
      deployment_policy:
        policy_type: selected_branches_and_tags
        branch_patterns: ["main", "release/*"]
        tag_patterns: ["v*"]

🔧 Implementation Details

Importer Changes:

Added FeatureGithubEnvironment constant
Feature check: cfg.IsFeatureEnabled(FeatureGithubEnvironment)
Fetches environment data from GitHub API including deployment policies
Generates YAML with new deployment_policy structure

Terraform Changes:

Dynamic environment resources based on YAML configuration
Handles deployment branch policies via separate resources
Import blocks for existing environments

📚 Documentation

feature_github_environment.md: Complete guide for environment configuration
ADDING_FEATURES.md: How to add new feature-flagged functionality
LOCAL_DEVELOPMENT_SETUP.md: Updated with environment examples
GITHUB_ACTIONS_WORKFLOWS.md: New comprehensive workflow documentation
DEVELOPERS_GUIDE.md: Environment field reference

How to Use

  1. Enable the feature:
    # import-config.yaml
    feature_github_environment: true
  2. Import existing repos with environments:
    just import-repo org/repo
  3. Or add environments to new repos:
    # repos/my-repo.yaml
    environments:
      - environment: production
        deployment_policy:
          policy_type: protected_branches

🔄 Migration

All existing YAML configurations updated to new structure
Backward compatibility not required (clean implementation)
Feature flag feature_github_environment: true controls environment import

Testing

✅ Feature flag system working (disabled by default)
✅ Environment import from GitHub API
✅ Terraform resource creation and management
✅ All documentation updated

Important Notes

Feature is opt-in via feature_github_environment: true
Teams in reviewers must have repository access first
Maximum 6 combined users + teams as reviewers (GitHub limitation)

This PR adds GitHub environment support as a new, feature-flagged capability that can be gradually rolled out without affecting existing workflows.

References

docs/DEVELOPERS_GUIDE.md

pavlovic-ivan · 2025-11-21T10:35:48Z

docs/DEVELOPERS_GUIDE.md

+
+Configure GitHub deployment environments with protection rules and reviewers.
+
+**Import Control**: Set `feature_github_environment: true` in `import-config.yaml` to import environments.


This should be in another document, something like IMPORTER_DEVELOPERS_GUIDE.md or in the LOCAL_DEVELOPMENT_SETUP.md (if it's not already there).

The DEVELOPERS_GUIDE.md should focus only on the fields allowed to be set in a repo yaml config file

docs/DEVELOPERS_GUIDE.md

pavlovic-ivan · 2025-11-21T10:40:49Z

docs/DEVELOPERS_GUIDE.md

+### Environment Fields
+
+- **`environment`**: *(required, string)* Environment name
+- **`wait_timer`**: *(optional, int)* Delay in seconds (max 43200)


Isn't this a value in minutes not seconds? Please verify

correct, it's in minutes (max is 30 days or 43200 minutes) - will update it
verifyuing on my org for demo1 repo and its actual config

are you ok with below edit?

Suggested change

- **`wait_timer`**: *(optional, int)* Delay in seconds (max 43200)

- **`wait_timer`**: *(optional, int)* Delay in minutes (max 43200 or 30 days)

Yeah, looks good

pavlovic-ivan · 2025-11-21T10:43:15Z

docs/FEATURE_GITHUB_ENVIRONMENT.md

I don't think this file is necessary. We have an example configuration file in the config template repo: https://github.com/G-Research/github-terraformer-configuration-template/blob/main/repos/repository.yaml.example

Consider moving information to the example

pavlovic-ivan · 2025-11-21T10:44:43Z

General remark: the name gcss-config-repo shouldn't be mentioned because that is arbitrary name of the configuration repo. And it's not how it's named in GR org. It can lead to confusion of new adopters

pavlovic-ivan · 2025-11-21T10:48:39Z

docs/GITHUB_ACTIONS_WORKFLOWS.md

Shouldn't be a part of this PR. Please move it to another PR that focuses on documentation. This one should be only about environments

pavlovic-ivan · 2025-11-21T10:48:57Z

docs/LOCAL_DEVELOPMENT_SETUP.md

Shouldn't be a part of this PR. Please move it to another PR that focuses on documentation. This one should be only about environments

pavlovic-ivan · 2025-11-21T10:49:26Z

docs/ci_flows.md

Shouldn't be a part of this PR. Please move it to another PR that focuses on documentation. This one should be only about environments

pavlovic-ivan · 2025-11-21T10:49:43Z

docs/workflows.md

Shouldn't be a part of this PR. Please move it to another PR that focuses on documentation. This one should be only about environments

pavlovic-ivan · 2025-11-21T10:52:12Z

feature/github-repo-importer/ADDING_FEATURES.md

Shouldn't be a part of this PR. Please move it to another PR that focuses on documentation. This one should be only about environments. Also, i don't get the purpose of this file. Maybe we go through this on a call?

pavlovic-ivan · 2025-11-21T11:00:18Z

feature/github-repo-provisioning/modules/terraform-github-repository/main.tf

Module modules/terraform-github-repository is vendored. That means it shouldn't be changed. All changes and features are added to the root module

pavlovic-ivan · 2025-11-21T11:00:31Z

feature/github-repo-provisioning/modules/terraform-github-repository/outputs.tf

Module modules/terraform-github-repository is vendored. That means it shouldn't be changed. All changes and features are added to the root module

pavlovic-ivan · 2025-11-21T11:00:40Z

feature/github-repo-provisioning/modules/terraform-github-repository/variables.tf

Module modules/terraform-github-repository is vendored. That means it shouldn't be changed. All changes and features are added to the root module

pavlovic-ivan · 2025-11-21T11:01:38Z

feature/github-repo-provisioning/main.tf

+  # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  # Environments Configuration
+  # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+  environments = try(each.value.environments, [])


Once environments are moved from the child module to the root module, this should be removed.

pavlovic-ivan · 2025-11-21T11:06:43Z

feature/github-repo-provisioning/main.tf

+import {
+  for_each = local.generated_environments_map
+
+  to = module.repository[each.value.repository].github_repository_environment.environment[each.value.environment.environment]


This will change once environments are moved to root module

pavlovic-ivan · 2025-11-21T11:28:41Z

feature/github-repo-importer/pkg/github/repositories.go

+type Environment struct {
+	Environment       string                `yaml:"environment"`
+	WaitTimer         *int                  `yaml:"wait_timer,omitempty"`
+	CanAdminsBypass   *bool                 `yaml:"can_admins_bypass,omitempty"`
+	PreventSelfReview *bool                 `yaml:"prevent_self_review,omitempty"` // Extracted from ProtectionRules in API response
+	Reviewers         *EnvironmentReviewers `yaml:"reviewers,omitempty"`
+	DeploymentPolicy  *DeploymentPolicy     `yaml:"deployment_policy,omitempty"`
+}
+
+type EnvironmentReviewers struct {
+	Teams []string `yaml:"teams,omitempty"` // Team slugs (e.g., "platform-team")
+	Users []string `yaml:"users,omitempty"` // GitHub usernames (e.g., "octocat")
+}
+
+// DeploymentPolicy represents the cleaner structure for deployment policies
+type DeploymentPolicy struct {
+	PolicyType     string   `yaml:"policy_type"`                // "protected_branches" or "selected_branches_and_tags"
+	BranchPatterns []string `yaml:"branch_patterns,omitempty"`  // e.g., ["release/*", "main"] - only for selected_branches_and_tags
+	TagPatterns    []string `yaml:"tag_patterns,omitempty"`     // e.g., ["v*"] - only for selected_branches_and_tags
+}


Please move this to environments.go. And remove comments

pavlovic-ivan · 2025-11-21T11:30:15Z

feature/github-repo-importer/pkg/github/github.go

+	// Log enabled features
+	if cfg != nil && cfg.Features != nil {
+		for featureName, enabled := range cfg.Features {
+			if enabled {
+				fmt.Printf("Feature enabled: %s\n", featureName)
+			}
+		}
+	}


Not needed, please remove

pavlovic-ivan · 2025-11-21T11:47:59Z

docs/DEVELOPERS_GUIDE.md

+  - **`users`**: *(string[])* GitHub usernames (max 6 total)
+  - **`teams`**: *(string[])* Team slugs (max 6 total)


It can be max 6 in total, not 6 per list

pavlovic-ivan · 2025-11-21T11:51:21Z

feature/github-repo-importer/pkg/github/constants.go

+	// Feature flags
+	// All feature flags follow the pattern: feature_<feature_name>
+	// Add new features here and they'll automatically work with the config system
+	FeatureGithubEnvironment = "feature_github_environment"


Suggested change

FeatureGithubEnvironment = "feature_github_environment"

FeatureGithubEnvironments = "feature_github_environments"

pavlovic-ivan · 2025-11-21T11:55:57Z

feature/github-repo-importer/pkg/github/config.go

+	IgnoredRepos  []string          `yaml:"ignored_repos,omitempty"`
+	SelectedRepos []string          `yaml:"selected_repos,omitempty"`
+	PageSize      *int              `yaml:"page_size,omitempty"`
+	Features      map[string]bool   `yaml:",inline"`


This line means that any field added to the root of the import-config.yaml file that has a boolean value will act as a feature flag, eg:

feature_github_environment: true some_ficticious_feature: true

Both will be captured. We want a strict configuration file that can be (de)serialized explicitly. Please change the Config struct accordingly

pavlovic-ivan · 2025-11-21T11:56:42Z

feature/github-repo-importer/pkg/github/constants.go

+	// Feature flags
+	// All feature flags follow the pattern: feature_<feature_name>
+	// Add new features here and they'll automatically work with the config system


This is not correct information. Please remove it. It's also affected by the comment https://github.com/G-Research/github-terraformer/pull/28/files#r2549525571

pavlovic-ivan · 2025-11-21T11:57:15Z

feature/github-repo-importer/pkg/github/constants.go

+	// Example future features:
+	// FeatureGithubWebhooks    = "feature_github_webhooks"
+	// FeatureGithubSecrets     = "feature_github_secrets"
+	// FeatureGithubTopics      = "feature_github_topics"


Please remove comments. Also, topics are already supported

pavlovic-ivan · 2025-11-21T11:59:30Z

feature/github-repo-importer/pkg/github/github.go

+	enableEnvironments := false
+	if cfg != nil && cfg.Features != nil {
+		if enabled, exists := cfg.Features[FeatureGithubEnvironment]; exists {
+			enableEnvironments = enabled
+		}
+	}
+
+	if enableEnvironments {


the three ifs can be reduced. you added a function that is not used cfg.IsFeatureEnabled that you can use to reduce the number of ifs. once that is done, you won't need enableEnvironments any more

pavlovic-ivan · 2025-11-21T12:06:30Z

feature/github-repo-importer/pkg/github/github.go

+			if err != nil {
+				if res != nil && res.StatusCode == http.StatusNotFound {
+					fmt.Printf("environments not found (this is normal for repositories without environments): %v\n", err)
+				} else {
+					fmt.Printf("failed to get environments: %v\n", err)
+				}
+				break
+			}


This should be changed so that we fail the import if getting the environments fail. Repo can not and shouldn't be partially imported, otherwise we will get drift in terraform. So it's "all or nothing" approach

pavlovic-ivan · 2025-11-21T12:06:47Z

feature/github-repo-importer/pkg/github/github.go

+						if err != nil {
+							fmt.Printf("Warning: failed to get full details for environment %s: %v\n", *env.Name, err)
+							// Use basic info if detailed fetch fails
+							allEnvironments = append(allEnvironments, env)
+						} else {
+							// Use full environment data with all details
+							allEnvironments = append(allEnvironments, fullEnv)
+						}


Similar as above, this should be changed so that we fail the import if getting the environments fail. Repo can not and shouldn't be partially imported, otherwise we will get drift in terraform. So it's "all or nothing" approach

pavlovic-ivan · 2025-11-21T12:14:10Z

feature/github-repo-importer/pkg/github/github.go

+	if err != nil {
+		fmt.Printf("Warning: failed to get organization info: %v\n", err)
+		return nil
+	}


This should be changed to a more go idiomatic approach, where you capture the error, log it, and escalate it above to the called function. Check the rest of the code to see how it's done, it's used everywhere. function resolveEnvironments will then return ([]Environment, error)

pavlovic-ivan · 2025-11-21T12:22:06Z

feature/github-repo-importer/pkg/github/github.go

+							continue
+						}
+
+						// The Reviewer field is an interface{} - try different type casts


Keep this one it's useful

pavlovic-ivan · 2025-11-21T12:23:10Z

feature/github-repo-importer/pkg/github/github.go

+									// Fallback: try map[string]interface{} for older API versions
+									if reviewerData, ok := reqReviewer.Reviewer.(map[string]interface{}); ok {
+										if login, ok := reviewerData["login"].(string); ok {
+											protectionReviewers.Users = append(protectionReviewers.Users, login)
+										}
+									}


i don't understand this. please explain

pavlovic-ivan · 2025-11-21T12:50:56Z

I will remove comments that say "remove comments", and leave only those that say "keep this comment" so that the number of asked changes here are reduced, but it also means that comments should be removed

pavlovic-ivan · 2025-11-21T12:55:45Z

feature/github-repo-importer/pkg/github/github.go

+			CanAdminsBypass: env.CanAdminsBypass,
+		}
+
+		// Extract PreventSelfReview, WaitTimer, and Reviewers from ProtectionRules


Keep this comment line, remove second and third

pavlovic-ivan · 2025-11-21T12:59:07Z

feature/github-repo-importer/pkg/github/github.go

+		// Handle reviewers at top level (fallback if not in ProtectionRules)
+		// API may return array of EnvReviewers with Type and ID
+		// We resolve IDs to human-readable names (usernames and team slugs)
+		// Note: This is usually empty as reviewers are typically in ProtectionRules
+		if env.Reviewers != nil && len(env.Reviewers) > 0 && environment.Reviewers == nil {
+			reviewers := &EnvironmentReviewers{}
+
+			// Separate reviewers by type and resolve IDs to names
+			for _, reviewer := range env.Reviewers {
+				if reviewer.Type != nil && reviewer.ID != nil {
+					switch *reviewer.Type {
+					case "Team":
+						// Resolve team ID to team slug
+						team, _, err := client.Teams.GetTeamByID(context.Background(), org.GetID(), *reviewer.ID)
+						if err != nil {
+							fmt.Printf("Warning: failed to resolve team ID %d: %v\n", *reviewer.ID, err)
+							continue
+						}
+						if team.Slug != nil {
+							reviewers.Teams = append(reviewers.Teams, *team.Slug)
+						}
+					case "User":
+						// Resolve user ID to username
+						user, _, err := client.Users.GetByID(context.Background(), *reviewer.ID)
+						if err != nil {
+							fmt.Printf("Warning: failed to resolve user ID %d: %v\n", *reviewer.ID, err)
+							continue
+						}
+						if user.Login != nil {
+							reviewers.Users = append(reviewers.Users, *user.Login)
+						}
+					}
+				}
+			}
+
+			if len(reviewers.Teams) > 0 || len(reviewers.Users) > 0 {
+				environment.Reviewers = reviewers
+			}
+		}


remove, unless you found a case where this is set. if there is one, then this block and the other on top can be merged, and reduced

pavlovic-ivan · 2025-11-21T13:01:44Z

feature/github-repo-importer/pkg/github/github.go

+			} else if env.DeploymentBranchPolicy.CustomBranchPolicies != nil && *env.DeploymentBranchPolicy.CustomBranchPolicies {
+				// Custom branch/tag patterns
+				deploymentPolicy.PolicyType = "selected_branches_and_tags"
+
+				// Fetch deployment branch policies from GitHub API
+				branchPatterns, tagPatterns := fetchDeploymentPolicies(client, owner, repo, env.GetName())
+
+				if len(branchPatterns) > 0 {
+					deploymentPolicy.BranchPatterns = branchPatterns
+				}
+				if len(tagPatterns) > 0 {
+					deploymentPolicy.TagPatterns = tagPatterns
+				}
+			}


This needs an else statement that should raise an error, because it means someone tampered with the json file and introduced something we don't know exists, or Github made a change, and we should be aware if any of the two happened. If so, throw an error, and fail the import.

pavlovic-ivan · 2025-11-21T13:02:50Z

feature/github-repo-importer/pkg/github/github.go

+				if len(branchPatterns) > 0 {
+					deploymentPolicy.BranchPatterns = branchPatterns
+				}
+				if len(tagPatterns) > 0 {
+					deploymentPolicy.TagPatterns = tagPatterns
+				}


These two should work together and check if length of both arrays in total is bigger than 6. If yes, throw an error, and fail the import.

pavlovic-ivan

Please take a look at comments and change accordingly

Co-authored-by: Ivan Pavlovic <[email protected]>

ljuboops257 and others added 4 commits November 18, 2025 19:37

test env

20bf48b

new structure

cbbc616

revert ljubon-org changes

1707869

update mermaid

333b31e

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

docs/DEVELOPERS_GUIDE.md Outdated Show resolved Hide resolved

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

docs/DEVELOPERS_GUIDE.md Outdated Show resolved Hide resolved

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

docs/DEVELOPERS_GUIDE.md Outdated Show resolved Hide resolved

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

docs/DEVELOPERS_GUIDE.md Outdated Show resolved Hide resolved

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

docs/DEVELOPERS_GUIDE.md Show resolved Hide resolved

pavlovic-ivan reviewed Nov 21, 2025

View reviewed changes

pavlovic-ivan requested changes Nov 21, 2025

View reviewed changes

ljuboops257 changed the title ~~GCSS0-1135: Add support for GitHub environment with deployment policy~~ GCSS-1135: Add support for GitHub environment with deployment policy Nov 24, 2025

ljuboops257 and others added 5 commits November 24, 2025 17:53

Update docs/DEVELOPERS_GUIDE.md

1142cfe

Co-authored-by: Ivan Pavlovic <[email protected]>

Update docs/DEVELOPERS_GUIDE.md

3be243d

Co-authored-by: Ivan Pavlovic <[email protected]>

Update docs/DEVELOPERS_GUIDE.md

097d297

Co-authored-by: Ivan Pavlovic <[email protected]>

Update docs/DEVELOPERS_GUIDE.md

62d7c47

Co-authored-by: Ivan Pavlovic <[email protected]>

Update docs/DEVELOPERS_GUIDE.md

c5f223c

Co-authored-by: Ivan Pavlovic <[email protected]>


		Configure GitHub deployment environments with protection rules and reviewers.

		Import Control: Set `feature_github_environment: true` in `import-config.yaml` to import environments.

	- `wait_timer`: (optional, int) Delay in seconds (max 43200)
	- `wait_timer`: (optional, int) Delay in minutes (max 43200 or 30 days)

		- `users`: (string[]) GitHub usernames (max 6 total)
		- `teams`: (string[]) Team slugs (max 6 total)

	FeatureGithubEnvironment = "feature_github_environment"
	FeatureGithubEnvironments = "feature_github_environments"

GCSS-1135: Add support for GitHub environment with deployment policy #28

Are you sure you want to change the base?

GCSS-1135: Add support for GitHub environment with deployment policy #28

Uh oh!

Conversation

ljuboops257 commented Nov 18, 2025

Add GitHub Environment Support with Improved Deployment Policy Structure

Key Changes

🚀 New Feature: GitHub Environments

🎛️ Feature Flag System

📝 Environment Configuration Structure

🔧 Implementation Details

How to Use

🔄 Migration

Testing

Important Notes

References

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pavlovic-ivan Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljuboops257 Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavlovic-ivan commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavlovic-ivan Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavlovic-ivan Nov 21, 2025 •

edited

Loading

ljuboops257 Nov 24, 2025 •

edited

Loading

pavlovic-ivan commented Nov 21, 2025 •

edited

Loading

pavlovic-ivan Nov 21, 2025 •

edited

Loading

pavlovic-ivan Nov 21, 2025 •

edited

Loading

pavlovic-ivan Nov 21, 2025 •

edited

Loading

pavlovic-ivan Nov 21, 2025 •

edited

Loading