fix: Ensure no duplicates in Global configuration and other fixes

cmd/gitlab_evaluate.go

@@ -31,7 +31,7 @@ func Evaluate(cCtx *cli.Context) error {
 			return err
 		}
 
-		ctx = config.WithConfig(ctx, globalCfg)
+		ctx = config.WithGlobalConfig(ctx, globalCfg)
 	}
 
 	cfg, err := config.LoadFile(state.ConfigFilePath(ctx))

cmd/gitlab_server.go

@@ -45,7 +45,7 @@ func Server(cCtx *cli.Context) error {
 			return err
 		}
 
-		ctx = config.WithConfig(ctx, globalCfg)
+		ctx = config.WithGlobalConfig(ctx, globalCfg)
 	}
 
 	//

cmd/gitlab_server_handlers.go

@@ -114,7 +114,8 @@ func GitLabWebhookHandler(ctx context.Context, webhookSecret string) http.Handle
 
 		// Check if there exists scm-config file in the repo before moving forward
 		file, err := client.MergeRequests().GetRemoteConfig(ctx, state.ConfigFilePath(ctx), state.CommitSHA(ctx))
-		if err != nil {
+		// only error when global config is not set
+		if err != nil && state.GlobalConfigFilePath(ctx) == "" {
 			errHandler(ctx, w, http.StatusOK, err)
 
 			return
@@ -125,7 +126,13 @@ func GitLabWebhookHandler(ctx context.Context, webhookSecret string) http.Handle
 		// In case of a parse error cfg remains "nil" and ProcessMR will try to read-and-parse it
 		// (but obviously also fail), but will surface the error within the GitLab External Pipeline (if enabled)
 		// which will surface the issue to the end-user directly
-		cfg, _ := config.ParseFile(file)
+		var cfg *config.Config
+		if file != nil { // file could be nil if no scm-config file is found when global config is set
+			cfg, _ = config.ParseFile(file)
+		} else {
+			// avoid trying to read-and-parse again if global config is set
+			cfg = config.GlobalConfigFromContext(ctx)
+		}
 
 		// Process the MR
 		if err := ProcessMR(ctx, client, cfg, fullEventPayload); err != nil {

cmd/shared.go

@@ -110,21 +110,20 @@ func ProcessMR(ctx context.Context, client scm.Client, cfg *config.Config, event
 
 		file, err := client.MergeRequests().GetRemoteConfig(ctx, state.ConfigFilePath(ctx), configSourceRef)
 		if err != nil {
-			return fmt.Errorf("could not read remote config file: %w", err)
-		}
-
-		// Parse the file
-		cfg, err = config.ParseFile(file)
-		if err != nil {
-			return fmt.Errorf("could not parse config file: %w", err)
+			slogctx.Warn(ctx, "Could not read remote config file", slog.Any("error", err))
+		} else {
+			// Parse the file
+			cfg, err = config.ParseFile(file)
+			if err != nil { // error on parsing failures when present
+				return fmt.Errorf("could not parse config file: %w", err)
+			}
 		}
 	}
 
 	// Merge previously loaded config with Repository config
-	ctxConfig := config.FromContext(ctx) // the global config if previously loaded
-	if ctxConfig != nil && cfg != nil {
-		ctxConfig.Merge(cfg)
-		cfg = ctxConfig
+	globalConfig := config.GlobalConfigFromContext(ctx) // the global config if previously loaded
+	if globalConfig != nil && cfg != nil {
+		cfg = globalConfig.Merge(cfg)
 	}
 
 	// Sanity check for having a configuration loaded

pkg/config/config.go

@@ -202,7 +202,11 @@ func (c *Config) Merge(other *Config) *Config {
 		for key, fileMap := range includes {
 			keyParts := strings.Split(key, ":")
 			project := keyParts[0]
-			ref := keyParts[1]
+
+			var ref *string
+			if refStr := keyParts[1]; refStr != "" {
+				ref = scm.Ptr(refStr)
+			}
 
 			files := make([]string, 0, len(fileMap))
 			for file := range fileMap {
@@ -211,7 +215,7 @@ func (c *Config) Merge(other *Config) *Config {
 
 			cfg.Includes = append(cfg.Includes, Include{
 				Project: project,
-				Ref:     scm.Ptr(ref),
+				Ref:     ref,
 				Files:   files,
 			})
 		}

pkg/config/context.go

@@ -8,6 +8,7 @@ type contextKey uint
 
 const (
 	configKey contextKey = iota
+	globalConfigKey
 )
 
 func WithConfig(ctx context.Context, config *Config) context.Context {
@@ -17,3 +18,11 @@ func WithConfig(ctx context.Context, config *Config) context.Context {
 func FromContext(ctx context.Context) *Config {
 	return ctx.Value(configKey).(*Config) //nolint:forcetypeassert
 }
+
+func WithGlobalConfig(ctx context.Context, config *Config) context.Context {
+	return context.WithValue(ctx, globalConfigKey, config)
+}
+
+func GlobalConfigFromContext(ctx context.Context) *Config {
+	return ctx.Value(globalConfigKey).(*Config) //nolint:forcetypeassert
+}

pkg/integration/backstage/client.go

@@ -111,10 +111,12 @@ func (c *Client) GetUser(ctx context.Context, userEntityRef *EntityReference) (*
 func (c *Client) ListGroupMembers(ctx context.Context, groupEntityRef *EntityReference) ([]go_backstage.Entity, error) {
 	var users []go_backstage.Entity
 
+	ref := groupEntityRef.ToString()
+
 	users, response, err := c.wrapped.Catalog.Entities.List(ctx, &backstage.ListEntityOptions{
 		Filters: []string{
 			// https://backstage.io/docs/features/software-catalog/well-known-relations/#memberof-and-hasmember
-			"kind=user,relations.memberof=" + groupEntityRef.ToString(),
+			"kind=user,relations.memberof=" + ref,
 		},
 	})
 	if err != nil {
@@ -222,6 +224,11 @@ func parseEntityReference(entityRef string) (*EntityReference, error) {
 		ref.Namespace = "default"
 	}
 
+	// default to group if kind is not set
+	if ref.Kind == "" {
+		ref.Kind = "group"
+	}
+
 	return ref, nil
 }