Extend the GitHub App configuration with an autoRegistration object (#…

…3449)

* Add the protobuf struct for generic provider options

In order to actually set the auto-registration of entities, we need to
be able to configure those options.

This patch adds two structures: AutoRegistration which contains a list
of entities to auto-register and ProviderConfig which contains the
AutoRegistration.

The idea behind this is that any provider would be able to use the
ProviderConfig attributes regardless of the provider type. So as an
example, both would be valid:

```
{ "auto_registration": { "enabled": ["repository"] }, "github": { "endpoint": "https://api.github.com" }}
```
and:
```
{ "auto_registration": { "enabled": ["repository"] }, "github-app": { "endpoint": "https://api.github.com" }}
```

and all would require just embedding the ProviderConfig structure into
the provider-specific parser.

Fixes: #3266

* Parse out ProviderConfig as well when reading GH App configuration

Amends the ParseV1AppConfig to also include parsing the
minderv1.GitHubAppProviderConfig structure.

The parsed-out object is not used yet, but it's parsed and validated and the
new code is tested.

Related: #3266

internal/providers/github/clients/app.go

@@ -111,22 +111,34 @@ func NewGitHubAppProvider(
 }
 
 // ParseV1AppConfig parses the raw config into a GitHubAppProviderConfig struct
-func ParseV1AppConfig(rawCfg json.RawMessage) (*minderv1.GitHubAppProviderConfig, error) {
+func ParseV1AppConfig(rawCfg json.RawMessage) (
+	*minderv1.ProviderConfig,
+	*minderv1.GitHubAppProviderConfig,
+	error,
+) {
+	// embedding the struct to expose its JSON tags
 	type wrapper struct {
+		*minderv1.ProviderConfig
 		GitHubApp *minderv1.GitHubAppProviderConfig `json:"github-app" yaml:"github-app" mapstructure:"github-app" validate:"required"`
 	}
 
 	var w wrapper
 	if err := provifv1.ParseAndValidate(rawCfg, &w); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	// Validate the config according to the protobuf validation rules.
 	if err := w.GitHubApp.Validate(); err != nil {
-		return nil, fmt.Errorf("error validating GitHubOAuth v1 provider config: %w", err)
+		return nil, nil, fmt.Errorf("error validating GitHubApp v1 provider config: %w", err)
+	}
+
+	if w.ProviderConfig != nil {
+		if err := w.ProviderConfig.Validate(); err != nil {
+			return nil, nil, fmt.Errorf("error validating provider config: %w", err)
+		}
 	}
 
-	return w.GitHubApp, nil
+	return w.ProviderConfig, w.GitHubApp, nil
 }
 
 // Ensure that the GitHubAppDelegate client implements the GitHub Delegate interface

internal/providers/github/clients/app_test.go

@@ -39,6 +39,86 @@ import (
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
 
+func TestParseV1AppConfig(t *testing.T) {
+	t.Parallel()
+
+	scenarios := []struct {
+		name       string
+		config     json.RawMessage
+		error      string
+		ghEvalFn   func(*testing.T, *minderv1.GitHubAppProviderConfig)
+		provEvalFn func(*testing.T, *minderv1.ProviderConfig)
+	}{
+		{
+			name:   "valid app config",
+			config: json.RawMessage(`{ "github-app": { "endpoint": "https://api.github.com" } }`),
+			ghEvalFn: func(t *testing.T, ghConfig *minderv1.GitHubAppProviderConfig) {
+				t.Helper()
+				assert.Equal(t, "https://api.github.com", ghConfig.Endpoint)
+			},
+			provEvalFn: func(t *testing.T, providerConfig *minderv1.ProviderConfig) {
+				t.Helper()
+				assert.Nil(t, providerConfig)
+			},
+		},
+		{
+			name:   "valid app and provider config",
+			config: json.RawMessage(`{ "auto_registration": { "enabled": ["repository"] }, "github-app": { "endpoint": "https://api.github.com" } }`),
+			ghEvalFn: func(t *testing.T, ghConfig *minderv1.GitHubAppProviderConfig) {
+				t.Helper()
+				assert.Equal(t, "https://api.github.com", ghConfig.Endpoint)
+			},
+			provEvalFn: func(t *testing.T, providerConfig *minderv1.ProviderConfig) {
+				t.Helper()
+				assert.Equal(t, []string{"repository"}, providerConfig.AutoRegistration.Enabled)
+			},
+		},
+		{
+			name:   "auto_registration does not validate the enabled entities",
+			config: json.RawMessage(`{ "auto_registration": { "enabled": ["beer"] } , "github-app": { "endpoint": "https://api.github.com" }}`),
+			ghEvalFn: func(t *testing.T, ghConfig *minderv1.GitHubAppProviderConfig) {
+				t.Helper()
+				assert.Nil(t, ghConfig)
+			},
+			provEvalFn: func(t *testing.T, providerConfig *minderv1.ProviderConfig) {
+				t.Helper()
+				assert.Nil(t, providerConfig)
+			},
+			error: "error validating provider config: auto_registration: invalid entity type: beer",
+		},
+		{
+			name:   "missing required github key",
+			config: json.RawMessage(`{ "auto_registration": { "enabled": ["repository"] } }`),
+			ghEvalFn: func(t *testing.T, ghConfig *minderv1.GitHubAppProviderConfig) {
+				t.Helper()
+				assert.Nil(t, ghConfig)
+			},
+			provEvalFn: func(t *testing.T, providerConfig *minderv1.ProviderConfig) {
+				t.Helper()
+				assert.Nil(t, providerConfig)
+			},
+			error: "Field validation for 'GitHubApp' failed on the 'required' tag",
+		},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			t.Parallel()
+
+			providerConfig, gitHubConfig, err := ParseV1AppConfig(scenario.config)
+			if scenario.error != "" {
+				assert.Error(t, err)
+				assert.Nil(t, providerConfig)
+				assert.Contains(t, err.Error(), scenario.error)
+			} else {
+				assert.NoError(t, err)
+				scenario.provEvalFn(t, providerConfig)
+				scenario.ghEvalFn(t, gitHubConfig)
+			}
+		})
+	}
+}
+
 func TestNewGitHubAppProvider(t *testing.T) {
 	t.Parallel()
 

internal/providers/github/clients/oauth_test.go

@@ -16,6 +16,7 @@ package clients
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
@@ -35,6 +36,44 @@ import (
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
 
+func TestParseV1OAuthConfig(t *testing.T) {
+	t.Parallel()
+
+	scenarios := []struct {
+		name     string
+		config   json.RawMessage
+		error    string
+		ghEvalFn func(*minderv1.GitHubProviderConfig)
+	}{
+		{
+			name:   "valid oauth config",
+			config: json.RawMessage(`{ "github": { "endpoint": "https://api.github.com" } }`),
+			ghEvalFn: func(ghConfig *minderv1.GitHubProviderConfig) {
+				assert.Equal(t, "https://api.github.com", ghConfig.Endpoint)
+			},
+		},
+		{
+			name:   "missing required github key",
+			config: json.RawMessage(`{ "auto_registration": { "enabled": ["repository"] } }`),
+			ghEvalFn: func(ghConfig *minderv1.GitHubProviderConfig) {
+				assert.Nil(t, ghConfig)
+			},
+			error: "Field validation for 'GitHub' failed on the 'required' tag",
+		},
+	}
+
+	for _, scenario := range scenarios {
+		gitHubConfig, err := ParseV1OAuthConfig(scenario.config)
+		if scenario.error != "" {
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), scenario.error)
+		} else {
+			assert.NoError(t, err)
+			scenario.ghEvalFn(gitHubConfig)
+		}
+	}
+}
+
 func TestNewRestClient(t *testing.T) {
 	t.Parallel()
 

internal/providers/github/manager/manager.go

@@ -128,7 +128,7 @@ func (g *githubProviderManager) Build(ctx context.Context, config *db.Provider)
 		return cli, nil
 	}
 
-	cfg, err := clients.ParseV1AppConfig(config.Definition)
+	_, cfg, err := clients.ParseV1AppConfig(config.Definition)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing github app config: %w", err)
 	}
@@ -263,7 +263,7 @@ func (g *githubProviderManager) createProviderWithInstallationToken(
 		return nil, fmt.Errorf("error getting installation ID: %w", err)
 	}
 
-	cfg, err := clients.ParseV1AppConfig(prov.Definition)
+	_, cfg, err := clients.ParseV1AppConfig(prov.Definition)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing github app config: %w", err)
 	}

internal/providers/providers.go

@@ -155,7 +155,7 @@ func getInstallationTokenCredential(
 	} else if err != nil {
 		return nil, fmt.Errorf("error getting installation ID: %w", err)
 	}
-	cfg, err := clients.ParseV1AppConfig(prov.Definition)
+	_, cfg, err := clients.ParseV1AppConfig(prov.Definition)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing github app config: %w", err)
 	}