Merge pull request #151 from Interhyp/cleanup_promoters

feat(RELTEC-11166): drop promoters logic

README.md

@@ -74,9 +74,6 @@ the [`local-config.yaml`][config] can be used to set the variables.
 | `ALERT_TARGET_PREFIX`              |                                                       | Validates the alert target to either match the prefix or suffix.                                                                                                                                                     |
 | `ALERT_TARGET_SUFFIX`              |                                                       |                                                                                                                                                                                                                      |
 |                                    |                                                       |                                                                                                                                                                                                                      |
-| `ADDITIONAL_PROMOTERS`             |                                                       | Promoters to be added for all services. Can be left empty, or contain a comma separated list of usernames                                                                                                            |
-| `ADDITIONAL_PROMOTERS_FROM_OWNERS` |                                                       | Owner aliases from which to get additional promoters to be added for all services. Can be left empty, or contain a comma separated list of owner aliases                                                             |
-|                                    |                                                       |                                                                                                                                                                                                                      |
 | `OWNER_ALIAS_PERMITTED_REGEX`      | `^[a-z](-?[a-z0-9]+)*$`                               | Regular expression to control the owner aliases that are permitted to be be created.                                                                                                                                 |
 | `OWNER_ALIAS_PROHIBITED_REGEX`     | `^$`                                                  | Regular expression to control the owner aliases that are prohibited to be be created.                                                                                                                                |
 | `OWNER_ALIAS_MAX_LENGTH`           | `28`                                                  | Maximum length of a valid owner alias.                                                                                                                                                                               |

acorns/config/customconfigint.go

@@ -44,9 +44,6 @@ type CustomConfiguration interface {
 	AlertTargetPrefix() string
 	AlertTargetSuffix() string
 
-	AdditionalPromoters() []string
-	AdditionalPromotersFromOwners() []string
-
 	ElasticApmEnabled() bool
 
 	OwnerAliasPermittedRegex() *regexp.Regexp
@@ -98,8 +95,6 @@ const (
 	KeyUpdateJobTimeoutSeconds        = "UPDATE_JOB_TIMEOUT_SECONDS"
 	KeyAlertTargetPrefix              = "ALERT_TARGET_PREFIX"
 	KeyAlertTargetSuffix              = "ALERT_TARGET_SUFFIX"
-	KeyAdditionalPromoters            = "ADDITIONAL_PROMOTERS"
-	KeyAdditionalPromotersFromOwners  = "ADDITIONAL_PROMOTERS_FROM_OWNERS"
 	KeyElasticApmDisabled             = "ELASTIC_APM_DISABLED"
 	KeyOwnerAliasPermittedRegex       = "OWNER_ALIAS_PERMITTED_REGEX"
 	KeyOwnerAliasProhibitedRegex      = "OWNER_ALIAS_PROHIBITED_REGEX"

acorns/service/ownersint.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+
 	openapi "github.com/Interhyp/metadata-service/api/v1"
 )
 
@@ -14,7 +15,6 @@ type Owners interface {
 	GetOwners(ctx context.Context) (openapi.OwnerListDto, error)
 	GetOwner(ctx context.Context, ownerAlias string) (openapi.OwnerDto, error)
 
-	RebuildPromoters(ctx context.Context, result *openapi.OwnerDto)
 	GetAllGroupMembers(ctx context.Context, groupOwner string, groupName string) []string
 
 	// CreateOwner returns the owner as it was created, with commit hash and timestamp filled in.

acorns/service/servicesint.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+
 	openapi "github.com/Interhyp/metadata-service/api/v1"
 )
 
@@ -31,14 +32,4 @@ type Services interface {
 	//
 	// Reason: they still need to be configured by bit-brother.
 	DeleteService(ctx context.Context, serviceName string, deletionInfo openapi.DeletionDto) error
-
-	// GetServicePromoters obtains the users who are allowed to promote a service.
-	//
-	// The promoters come from
-	// - the promoters field in the owner info for the given owner alias
-	// - ALL productOwners
-	// - the promoters field for any owner alias listed in the configuration (like it admins)
-	//
-	// The list is sorted and made unique.
-	GetServicePromoters(ctx context.Context, serviceOwnerAlias string) (openapi.ServicePromotersDto, error)
 }

docs/local-config.template.yaml

@@ -25,7 +25,9 @@ VAULT_SECRETS_CONFIG: >-
       {"vaultKey": "BASIC_AUTH_USERNAME"},
       {"vaultKey": "BASIC_AUTH_PASSWORD"},
       {"vaultKey": "BITBUCKET_PASSWORD"},
-      {"vaultKey": "KAFKA_PASSWORD"}
+      {"vaultKey": "KAFKA_PASSWORD"},
+      {"vaultKey": "SSH_PRIVATE_KEY"},
+      {"vaultKey": "SSH_PRIVATE_KEY_PASSWORD"}
     ]
   }
 

internal/repository/config/accessors.go

@@ -114,14 +114,6 @@ func (c *CustomConfigImpl) AlertTargetSuffix() string {
 	return c.VAlertTargetSuffix
 }
 
-func (c *CustomConfigImpl) AdditionalPromoters() []string {
-	return strings.Split(c.VAdditionalPromoters, ",")
-}
-
-func (c *CustomConfigImpl) AdditionalPromotersFromOwners() []string {
-	return strings.Split(c.VAdditionalPromotersFromOwners, ",")
-}
-
 func (c *CustomConfigImpl) ElasticApmEnabled() bool {
 	return !c.VElasticApmDisabled &&
 		os.Getenv("ELASTIC_APM_SERVER_URL") != "" &&

internal/repository/config/config.go

@@ -196,20 +196,6 @@ var CustomConfigItems = []auconfigapi.ConfigItem{
 		Default:  "",
 		Validate: auconfigenv.ObtainPatternValidator("^@[a-z0-9-]+.[a-z]{2,3}$"),
 	},
-	{
-		Key:         config.KeyAdditionalPromoters,
-		EnvName:     config.KeyAdditionalPromoters,
-		Default:     "",
-		Description: "promoters to be added for all services. Can be left empty, or contain a comma separated list of usernames",
-		Validate:    auconfigenv.ObtainPatternValidator("^|[a-z](-?[a-z0-9]+)*(,[a-z](-?[a-z0-9]+)*)*$"),
-	},
-	{
-		Key:         config.KeyAdditionalPromotersFromOwners,
-		EnvName:     config.KeyAdditionalPromotersFromOwners,
-		Default:     "",
-		Description: "owner aliases from which to get additional promoters to be added for all services. Can be left empty, or contain a comma separated list of owner aliases",
-		Validate:    auconfigenv.ObtainPatternValidator("^|[a-z](-?[a-z0-9]+)*(,[a-z](-?[a-z0-9]+)*)*$"),
-	},
 	{
 		Key:         config.KeyElasticApmDisabled,
 		EnvName:     config.KeyElasticApmDisabled,

internal/repository/config/plumbing.go

@@ -40,8 +40,6 @@ type CustomConfigImpl struct {
 	VUpdateJobTimeoutSeconds        uint16
 	VAlertTargetPrefix              string
 	VAlertTargetSuffix              string
-	VAdditionalPromoters            string
-	VAdditionalPromotersFromOwners  string
 	VElasticApmDisabled             bool
 	VOwnerAliasPermittedRegex       *regexp.Regexp
 	VOwnerAliasProhibitedRegex      *regexp.Regexp
@@ -93,8 +91,6 @@ func (c *CustomConfigImpl) Obtain(getter func(key string) string) {
 	c.VUpdateJobTimeoutSeconds = toUint16(getter(config.KeyUpdateJobTimeoutSeconds))
 	c.VAlertTargetPrefix = getter(config.KeyAlertTargetPrefix)
 	c.VAlertTargetSuffix = getter(config.KeyAlertTargetSuffix)
-	c.VAdditionalPromoters = getter(config.KeyAdditionalPromoters)
-	c.VAdditionalPromotersFromOwners = getter(config.KeyAdditionalPromotersFromOwners)
 	c.VElasticApmDisabled, _ = strconv.ParseBool(getter(config.KeyElasticApmDisabled))
 	c.VOwnerAliasPermittedRegex, _ = regexp.Compile(getter(config.KeyOwnerAliasPermittedRegex))
 	c.VOwnerAliasProhibitedRegex, _ = regexp.Compile(getter(config.KeyOwnerAliasProhibitedRegex))

internal/repository/config/validation_test.go

@@ -134,8 +134,6 @@ func TestAccessors(t *testing.T) {
 	require.Equal(t, uint16(30), config.Custom(cut).UpdateJobTimeoutSeconds())
 	require.Equal(t, "https://some-domain.com/", config.Custom(cut).AlertTargetPrefix())
 	require.Equal(t, "@some-domain.com", config.Custom(cut).AlertTargetSuffix())
-	require.EqualValues(t, []string{"someguy"}, config.Custom(cut).AdditionalPromoters())
-	require.EqualValues(t, []string{"add-my-promoters-to-every-service", "also-add-my-promoters"}, config.Custom(cut).AdditionalPromotersFromOwners())
 	require.Equal(t, "[a-z][0-1]+", config.Custom(cut).OwnerAliasPermittedRegex().String())
 	require.Equal(t, "[a-z][0-2]+", config.Custom(cut).OwnerAliasProhibitedRegex().String())
 	require.Equal(t, uint16(1), config.Custom(cut).OwnerAliasMaxLength())

internal/service/mapper/owner.go

@@ -2,9 +2,10 @@ package mapper
 
 import (
 	"context"
+	"sort"
+
 	openapi "github.com/Interhyp/metadata-service/api/v1"
 	"github.com/Interhyp/metadata-service/internal/service/util"
-	"sort"
 )
 
 func (s *Impl) GetSortedOwnerAliases(_ context.Context) ([]string, error) {
@@ -38,8 +39,6 @@ func (s *Impl) GetOwner(ctx context.Context, ownerAlias string) (openapi.OwnerDt
 	err := GetT[openapi.OwnerDto](ctx, s, &result, fullPath)
 
 	if nil == err {
-		s.processPromoters(ctx, &result)
-
 		if result.Groups != nil {
 			s.processGroupMap(ctx, result.Groups)
 		}
@@ -48,27 +47,6 @@ func (s *Impl) GetOwner(ctx context.Context, ownerAlias string) (openapi.OwnerDt
 	return result, err
 }
 
-func (s *Impl) processPromoters(ctx context.Context, result *openapi.OwnerDto) {
-	users, groups := util.SplitUsersAndGroups(result.Promoters)
-	if len(users) > 0 {
-		filteredExistingUsers, err2 := s.Bitbucket.FilterExistingUsernames(ctx, users)
-		if err2 == nil {
-			userDifference := util.Difference(users, filteredExistingUsers)
-			if len(userDifference) > 0 {
-				s.Logging.Logger().Ctx(ctx).Error().Printf("Found unknown users in configuration: %v", userDifference)
-			}
-			result.Promoters = append(filteredExistingUsers, groups...)
-		} else {
-			s.Logging.Logger().Ctx(ctx).Error().Printf("Error checking existing bitbucket users: %s", err2.Error())
-		}
-
-		if len(result.Promoters) <= 0 && len(users) > 0 {
-			s.Logging.Logger().Ctx(ctx).Warn().Printf("Fallback to predefined reviewers")
-			result.Promoters = append(result.Promoters, s.CustomConfiguration.BitbucketReviewerFallback())
-		}
-	}
-}
-
 func (s *Impl) processGroupMap(ctx context.Context, groupsMap *map[string][]string) {
 	for groupName, groupMembers := range *groupsMap {
 		users, groups := util.SplitUsersAndGroups(groupMembers)

internal/service/owners/owners.go

@@ -3,12 +3,12 @@ package owners
 import (
 	"context"
 	"fmt"
+	"strings"
+
 	"github.com/Interhyp/metadata-service/acorns/service"
 	openapi "github.com/Interhyp/metadata-service/api/v1"
-	"github.com/Interhyp/metadata-service/internal/service/util"
 	librepo "github.com/StephanHCB/go-backend-service-common/acorns/repository"
 	"github.com/StephanHCB/go-backend-service-common/api/apierrors"
-	"strings"
 )
 
 type Impl struct {
@@ -40,30 +40,7 @@ func (s *Impl) GetOwners(ctx context.Context) (openapi.OwnerListDto, error) {
 }
 
 func (s *Impl) GetOwner(ctx context.Context, ownerAlias string) (openapi.OwnerDto, error) {
-	owner, err := s.Cache.GetOwner(ctx, ownerAlias)
-
-	if err == nil {
-		s.RebuildPromoters(ctx, &owner)
-	}
-
-	return owner, err
-}
-
-func (s *Impl) RebuildPromoters(ctx context.Context, result *openapi.OwnerDto) {
-	if result == nil {
-		return
-	}
-	filteredPromoters := make([]string, 0)
-	for _, promoter := range result.Promoters {
-		isGroup, groupOwner, groupName := util.ParseGroupOwnerAndGroupName(promoter)
-		if isGroup {
-			groupMembers := s.GetAllGroupMembers(ctx, groupOwner, groupName)
-			filteredPromoters = append(filteredPromoters, groupMembers...)
-		} else {
-			filteredPromoters = append(filteredPromoters, promoter)
-		}
-	}
-	result.Promoters = util.RemoveDuplicateStr(filteredPromoters)
+	return s.Cache.GetOwner(ctx, ownerAlias)
 }
 
 func (s *Impl) GetAllGroupMembers(ctx context.Context, groupOwner string, groupName string) []string {
@@ -116,7 +93,6 @@ func (s *Impl) mapOwnerCreateDtoToOwnerDto(ownerCreateDto openapi.OwnerCreateDto
 		ProductOwner:       ownerCreateDto.ProductOwner,
 		JiraIssue:          ownerCreateDto.JiraIssue,
 		DefaultJiraProject: ownerCreateDto.DefaultJiraProject,
-		Promoters:          ownerCreateDto.Promoters,
 		Groups:             ownerCreateDto.Groups,
 	}
 }
@@ -258,7 +234,6 @@ func patchOwner(current openapi.OwnerDto, patch openapi.OwnerPatchDto) openapi.O
 		Contact:            patchString(patch.Contact, current.Contact),
 		ProductOwner:       patchStringPtr(patch.ProductOwner, current.ProductOwner),
 		DefaultJiraProject: patchStringPtr(patch.DefaultJiraProject, current.DefaultJiraProject),
-		Promoters:          patchStringArray(patch.Promoters, current.Promoters),
 		Groups:             patchStringToStringArrayMapPtr(patch.Groups, current.Groups),
 		TimeStamp:          patch.TimeStamp,
 		CommitHash:         patch.CommitHash,
@@ -287,14 +262,6 @@ func patchString(patch *string, original string) string {
 	}
 }
 
-func patchStringArray(patch []string, original []string) []string {
-	if len(patch) == 0 {
-		return original
-	} else {
-		return patch
-	}
-}
-
 func patchStringToStringArrayMapPtr(patch *map[string][]string, original *map[string][]string) *map[string][]string {
 	if patch == nil {
 		return original

internal/service/services/services.go

@@ -4,13 +4,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
+
 	"github.com/Interhyp/metadata-service/acorns/config"
 	"github.com/Interhyp/metadata-service/acorns/service"
 	openapi "github.com/Interhyp/metadata-service/api/v1"
 	librepo "github.com/StephanHCB/go-backend-service-common/acorns/repository"
 	"github.com/StephanHCB/go-backend-service-common/api/apierrors"
-	"sort"
-	"strings"
 )
 
 type Impl struct {
@@ -410,68 +410,6 @@ func (s *Impl) validateDeletionDto(ctx context.Context, deletionInfo openapi.Del
 	return nil
 }
 
-func (s *Impl) GetServicePromoters(ctx context.Context, serviceOwnerAlias string) (openapi.ServicePromotersDto, error) {
-	resultSet := make(map[string]bool)
-
-	// add default promoters
-	err := s.addDefaultPromoters(ctx, resultSet)
-	if err != nil {
-		return openapi.ServicePromotersDto{}, err
-	}
-
-	// add the promoters for the given ownerAlias
-	err = s.addPromotersForOwner(ctx, serviceOwnerAlias, resultSet)
-	if err != nil {
-		return openapi.ServicePromotersDto{}, err
-	}
-
-	// add any extra promoters according to configuration
-	for _, additionalOwnerAlias := range s.CustomConfiguration.AdditionalPromotersFromOwners() {
-		err := s.addPromotersForOwner(ctx, additionalOwnerAlias, resultSet)
-		if err != nil {
-			return openapi.ServicePromotersDto{}, err
-		}
-	}
-
-	// add all product owners from all owners
-	err = s.addAllProductOwners(ctx, resultSet)
-	if err != nil {
-		return openapi.ServicePromotersDto{}, err
-	}
-
-	// sorted unique user list
-	result := make([]string, 0)
-	for user := range resultSet {
-		result = append(result, user)
-	}
-	sort.Strings(result)
-
-	return openapi.ServicePromotersDto{Promoters: result}, nil
-}
-
-func (s *Impl) addDefaultPromoters(ctx context.Context, resultSet map[string]bool) error {
-	for _, user := range s.CustomConfiguration.AdditionalPromoters() {
-		resultSet[user] = true
-	}
-	return nil
-}
-
-func (s *Impl) addPromotersForOwner(ctx context.Context, ownerAlias string, resultSet map[string]bool) error {
-	serviceOwner, err := s.Cache.GetOwner(ctx, ownerAlias)
-	if err != nil {
-		if !apierrors.IsNotFoundError(err) {
-			// concurrent cache update -> ok
-			return err
-		}
-	} else {
-		s.Owner.RebuildPromoters(ctx, &serviceOwner)
-		for _, user := range serviceOwner.Promoters {
-			resultSet[user] = true
-		}
-	}
-	return nil
-}
-
 func (s *Impl) addAllProductOwners(ctx context.Context, resultSet map[string]bool) error {
 	for _, alias := range s.Cache.GetSortedOwnerAliases(ctx) {
 		owner, err := s.Cache.GetOwner(ctx, alias)

internal/web/controller/servicectl/servicectl.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
+
 	"github.com/Interhyp/metadata-service/acorns/config"
 	"github.com/Interhyp/metadata-service/acorns/service"
 	openapi "github.com/Interhyp/metadata-service/api/v1"
@@ -12,7 +14,6 @@ import (
 	librepo "github.com/StephanHCB/go-backend-service-common/acorns/repository"
 	"github.com/StephanHCB/go-backend-service-common/api/apierrors"
 	"github.com/go-chi/chi/v5"
-	"net/http"
 )
 
 const ownerParam = "owner"
@@ -193,16 +194,11 @@ func (c *Impl) GetServicePromoters(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	serviceName := util.StringPathParam(r, "service")
 
-	serviceDto, err := c.Services.GetService(ctx, serviceName)
+	_, err := c.Services.GetService(ctx, serviceName)
 	if err != nil {
 		apierrors.HandleError(ctx, w, r, err, apierrors.IsNotFoundError)
 	} else {
-		promotersDto, err := c.Services.GetServicePromoters(ctx, serviceDto.Owner)
-		if err != nil {
-			util.UnexpectedErrorHandler(ctx, w, r, err, c.Timestamp.Now())
-		} else {
-			util.Success(ctx, w, r, promotersDto)
-		}
+		util.Success(ctx, w, r, openapi.ServicePromotersDto{Promoters: []string{}})
 	}
 }