Skip to content

Commit

Permalink
feat(RELTEC-12197): remove user validation to increase startup time, …
Browse files Browse the repository at this point in the history
…validation should be done by upstream services
  • Loading branch information
Knupfer, Raffael committed Feb 13, 2025
1 parent 855a7ef commit 0cc3c05
Show file tree
Hide file tree
Showing 10 changed files with 4 additions and 148 deletions.
2 changes: 0 additions & 2 deletions internal/acorn/config/customconfigint.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ type CustomConfiguration interface {

VCSConfigs() map[string]VCSConfig
WebhooksProcessAsync() bool
UserPrefix() string

Kafka() *kafka.Config
KafkaGroupIdOverride() string
Expand Down Expand Up @@ -142,5 +141,4 @@ const (
KeyPullRequestBuildKey = "PULL_REQUEST_BUILD_KEY"
KeyVCSConfigs = "VCS_CONFIGS"
KeyWebhooksProcessAsync = "WEBHOOKS_PROCESS_ASYNC"
KeyUserPrefix = "USER_PREFIX"
)
2 changes: 0 additions & 2 deletions internal/acorn/repository/vcsint.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ type VcsPlugin interface {
CreatePullRequestComment(ctx context.Context, repoPath, repoName, pullRequestID, text string) error

GetChangedFilesOnPullRequest(ctx context.Context, repoPath, repoName, pullRequestID, toRef string) ([]File, string, error)

GetUser(ctx context.Context, username string) (string, error)
}

type CommitBuildStatus string
Expand Down
8 changes: 0 additions & 8 deletions internal/client/bitbucket/bitbucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,3 @@ func (r *Impl) getFileContentsAt(ctx context.Context, repoPath string, repoName

return contents.String(), nil
}

func (r *Impl) GetUser(ctx context.Context, username string) (string, error) {
response, _, err := r.userApi.GetUser(ctx, username)
if err != nil {
return "", err
}
return *response.Name, nil
}
11 changes: 1 addition & 10 deletions internal/client/github/client.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
package githubclient

import (
"fmt"
"github.com/Interhyp/metadata-service/internal/acorn/config"
"github.com/google/go-github/v66/github"
"net/http"
"regexp"
)

func NewClient(client *http.Client, accessToken string, customConfig config.CustomConfiguration) (*github.Client, error) {
matcher := regexp.MustCompile(fmt.Sprintf("/users/[a-z_-]*%s", customConfig.UserPrefix()))
cacheClient := NewCaching(
client,
func(method string, url string, requestBody interface{}) bool {
return method == http.MethodGet && matcher.MatchString(url)
},
).Client()
return github.NewClient(cacheClient).WithAuthToken(accessToken), nil
return github.NewClient(client).WithAuthToken(accessToken), nil
}
14 changes: 0 additions & 14 deletions internal/client/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package githubclient

import (
"context"
"fmt"
"github.com/Interhyp/metadata-service/internal/acorn/config"
"github.com/Interhyp/metadata-service/internal/acorn/repository"
"github.com/Interhyp/metadata-service/internal/util"
"github.com/google/go-github/v66/github"
"strconv"
"strings"
)

type Impl struct {
Expand Down Expand Up @@ -89,15 +87,3 @@ func (r *Impl) GetChangedFilesOnPullRequest(ctx context.Context, repoPath, repoN
}
return result, toRef, nil
}

func (r *Impl) GetUser(ctx context.Context, username string) (string, error) {
prefix := r.CustomConfig.UserPrefix()
user, _, err := r.client.Users.Get(ctx, fmt.Sprintf("%s%s", username, prefix))
if err != nil {
return "", err
}
if prefix != "" {
return strings.Split(*user.Login, prefix)[0], nil
}
return *user.Login, nil
}
4 changes: 0 additions & 4 deletions internal/repository/config/accessors.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,3 @@ func (c *CustomConfigImpl) VCSConfigs() map[string]config.VCSConfig {
func (c *CustomConfigImpl) WebhooksProcessAsync() bool {
return c.VWebhooksProcessAsync
}

func (c *CustomConfigImpl) UserPrefix() string {
return c.VUserPrefix
}
7 changes: 0 additions & 7 deletions internal/repository/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,4 @@ var CustomConfigItems = []auconfigapi.ConfigItem{
Default: "true",
Validate: auconfigapi.ConfigNeedsNoValidation,
},
{
Key: config.KeyUserPrefix,
EnvName: config.KeyUserPrefix,
Description: "User prefix",
Default: "",
Validate: auconfigapi.ConfigNeedsNoValidation,
},
}
2 changes: 0 additions & 2 deletions internal/repository/config/plumbing.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ type CustomConfigImpl struct {
VPullRequestBuildKey string
VVCSConfig map[string]config.VCSConfig
VWebhooksProcessAsync bool
VUserPrefix string

VKafkaConfig *kafka.Config
GitUrlMatcher *regexp.Regexp
Expand Down Expand Up @@ -125,7 +124,6 @@ func (c *CustomConfigImpl) Obtain(getter func(key string) string) {
c.VPullRequestBuildKey = getter(config.KeyPullRequestBuildKey)
c.VVCSConfig, _ = parseVCSConfigs(getter(config.KeyVCSConfigs), getter)
c.VWebhooksProcessAsync, _ = toBoolean(getter(config.KeyWebhooksProcessAsync))
c.VUserPrefix = getter(config.KeyUserPrefix)

c.VKafkaConfig.Obtain(getter)
}
Expand Down
91 changes: 2 additions & 89 deletions internal/service/mapper/owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,9 @@ package mapper

import (
"context"
"fmt"
"github.com/Interhyp/metadata-service/api"
"github.com/Interhyp/metadata-service/internal/acorn/errors/httperror"
"github.com/Interhyp/metadata-service/internal/acorn/repository"
"net/http"
"sort"
"strings"

"github.com/Interhyp/metadata-service/internal/service/util"
"sort"
)

func (s *Impl) GetSortedOwnerAliases(_ context.Context) ([]string, error) {
Expand Down Expand Up @@ -56,16 +50,7 @@ func (s *Impl) processGroupMap(ctx context.Context, groupsMap map[string][]strin
for groupName, groupMembers := range groupsMap {
users, groups := util.SplitUsersAndGroups(groupMembers)
if len(users) > 0 {
filteredExistingUsers, err := s.filterExistingUsernames(ctx, users)
if err == nil {
userDifference := util.Difference(users, filteredExistingUsers)
if len(userDifference) > 0 {
s.Logging.Logger().Ctx(ctx).Warn().Printf("Found unknown users in configuration: %v", userDifference)
}
groupsMap[groupName] = append(filteredExistingUsers, groups...)
} else {
s.Logging.Logger().Ctx(ctx).Error().Printf("Error checking existing vcs users: %s", err.Error())
}
groupsMap[groupName] = append(users, groups...)

if len(groupsMap[groupName]) <= 0 && len(users) > 0 {
s.Logging.Logger().Ctx(ctx).Warn().Printf("Fallback to predefined reviewers")
Expand Down Expand Up @@ -122,75 +107,3 @@ func (s *Impl) IsOwnerEmpty(_ context.Context, ownerAlias string) bool {

return true
}

func (s *Impl) filterExistingUsernames(ctx context.Context, usernames []string) ([]string, error) {
existingUsernames := make([]string, 0)

dedupUsers := Unique(usernames)
existingUsers, err := s.getExisingUsers(ctx, dedupUsers)
if err != nil {
return existingUsernames, err
}

for _, user := range existingUsers {
existingUsernames = append(existingUsernames, user)
}
sort.Strings(existingUsernames)

return existingUsernames, nil
}

func Unique[T comparable](sliceList []T) []T {
allKeys := make(map[T]bool)
list := make([]T, 0)
for _, item := range sliceList {
if _, value := allKeys[item]; !value {
allKeys[item] = true
list = append(list, item)
}
}
return list
}

func (s *Impl) getExisingUsers(ctx context.Context, usernames []string) ([]string, error) {
users := make([]string, 0)
vcs, err := getMatchingVCS(s)
if err != nil {
return []string{}, err
}

for _, username := range usernames {
bbUser, err := vcs.GetUser(ctx, username)
if err != nil {
if httperror.Is(err) && err.(*httperror.Impl).Status() == http.StatusNotFound ||
strings.Contains(err.Error(), "404") {
s.Logging.Logger().Ctx(ctx).Warn().Printf("bitbucket user %s does not exist", username)
continue
}
s.Logging.Logger().Ctx(ctx).Error().WithErr(err).Print(fmt.Sprintf("failed to read bitbucket user %s", username))
return []string{}, err
}
users = append(users, bbUser)
}
return users, nil
}

func getMatchingVCS(s *Impl) (repository.VcsPlugin, error) {
vcs := new(repository.VcsPlugin)
if strings.Contains(s.CustomConfiguration.MetadataRepoUrl(), "github.com") {
platform, ok := s.VcsPlatforms["github"]
if !ok {
return nil, fmt.Errorf("github vcs not configured")
}
vcs = &platform.VCS
} else if strings.Contains(s.CustomConfiguration.MetadataRepoUrl(), "bitbucket") {
platform, ok := s.VcsPlatforms["bitbucket"]
if !ok {
return nil, fmt.Errorf("bitbucket vcs not configured")
}
vcs = &platform.VCS
} else {
return nil, fmt.Errorf("unknown vcs for repository url %s", s.CustomConfiguration.MetadataRepoUrl())
}
return *vcs, nil
}
11 changes: 1 addition & 10 deletions internal/service/mapper/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,7 @@ func (s *Impl) GetRepository(ctx context.Context, repoKey string) (openapi.Repos
for approversGroupName, approversGroup := range approversGroupsMap {
users, groups := util.SplitUsersAndGroups(approversGroup)
if len(users) > 0 {
filteredExistingUsers, err2 := s.filterExistingUsernames(ctx, users)
if err2 == nil {
userDifference := util.Difference(users, filteredExistingUsers)
if len(userDifference) > 0 {
s.Logging.Logger().Ctx(ctx).Warn().Printf("Found unknown users in configuration: %v", userDifference)
}
approversGroupsMap[approversGroupName] = append(filteredExistingUsers, groups...)
} else {
s.Logging.Logger().Ctx(ctx).Error().Printf("Error checking existing vcs users: %s", err2.Error())
}
approversGroupsMap[approversGroupName] = append(users, groups...)

if len(approversGroupsMap[approversGroupName]) <= 0 && len(users) > 0 {
s.Logging.Logger().Ctx(ctx).Warn().Printf("Fallback to predefined reviewers")
Expand Down

0 comments on commit 0cc3c05

Please sign in to comment.