Skip to content

Commit

Permalink
Merge pull request #363 from Interhyp/RELTEC-12294
Browse files Browse the repository at this point in the history
perf(RELTEC-12294): remove file category support
  • Loading branch information
KRaffael authored Nov 29, 2024
2 parents b20288a + e1aaaa4 commit 844e03e
Show file tree
Hide file tree
Showing 24 changed files with 16 additions and 175 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ the [`local-config.yaml`][config] can be used to set the variables.
| `REPOSITORY_TYPES` | | Comma separated list of supported repository types. |
| `REPOSITORY_KEY_SEPARATOR` | `.` | Single character used to separate repository name from repository type. repository name and repository type must not contain separator. |
| | | |
| `ALLOWED_FILE_CATEGORIES` | | List of allowed keys for the filecategory field in repositories. Parsed as a json array, example value: `["key1","key2"]`. All keys not in this list are rejected on writes, and silently dropped when reading. |
| | | |
| `REDIS_URL` | | Url to an optional Redis instance to use as a shared cache. Will use in-memory cache if left blank |
| `REDIS_PASSWORD` | | Password for the Redis instance. Can be read from Vault via `VAULT_SECRETS_CONFIG` |
Expand Down
2 changes: 0 additions & 2 deletions api/generated_model_repository_create_dto.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions api/generated_model_repository_dto.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions api/generated_model_repository_patch_dto.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 0 additions & 17 deletions api/openapi-v3-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1820,8 +1820,6 @@ components:
default: false
configuration:
$ref: '#/components/schemas/RepositoryConfigurationDto'
filecategory:
$ref: '#/components/schemas/RepositoryFileCategoriesDto'
timeStamp:
description: 'ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates.'
type: string
Expand Down Expand Up @@ -1875,8 +1873,6 @@ components:
default: false
configuration:
$ref: '#/components/schemas/RepositoryConfigurationDto'
filecategory:
$ref: '#/components/schemas/RepositoryFileCategoriesDto'
jiraIssue:
description: 'The jira issue to use for committing a change, or the last jira issue used.'
type: string
Expand Down Expand Up @@ -1918,8 +1914,6 @@ components:
default: false
configuration:
$ref: '#/components/schemas/RepositoryConfigurationPatchDto'
filecategory:
$ref: '#/components/schemas/RepositoryFileCategoriesDto'
timeStamp:
description: 'ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates.'
type: string
Expand Down Expand Up @@ -2235,17 +2229,6 @@ components:
properties:
text:
type: string
RepositoryFileCategoriesDto:
description: 'Assign a category to a list of files, e.g. to mark them for caching purposes. The key is the category name, and the value is a list of paths. Files are considered to have that category if their path is in the list.'
type: object
examples:
- cache-template:
- templates/template1.yaml
- another/path/template2.json
additionalProperties:
type: array
items:
type: string
ConditionReferenceDto:
description: Configuration of conditional build references.
type: object
Expand Down
3 changes: 0 additions & 3 deletions internal/acorn/config/customconfigint.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ type CustomConfiguration interface {

NotificationConsumerConfigs() map[string]NotificationConsumerConfig

AllowedFileCategories() []string

VCSConfigs() map[string]VCSConfig
WebhooksProcessAsync() bool
UserPrefix() string
Expand Down Expand Up @@ -138,7 +136,6 @@ const (
KeyRepositoryKeySeparator = "REPOSITORY_KEY_SEPARATOR"
KeyRepositoryTypes = "REPOSITORY_TYPES"
KeyNotificationConsumerConfigs = "NOTIFICATION_CONSUMER_CONFIGS"
KeyAllowedFileCategories = "ALLOWED_FILE_CATEGORIES"
KeyRedisUrl = "REDIS_URL"
KeyRedisPassword = "REDIS_PASSWORD"
KeyPullRequestBuildUrl = "PULL_REQUEST_BUILD_URL"
Expand Down
4 changes: 0 additions & 4 deletions internal/repository/config/accessors.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,6 @@ func (c *CustomConfigImpl) NotificationConsumerConfigs() map[string]config.Notif
return c.VNotificationConsumerConfigs
}

func (c *CustomConfigImpl) AllowedFileCategories() []string {
return c.VAllowedFileCategories
}

func (c *CustomConfigImpl) Kafka() *kafka.Config {
return c.VKafkaConfig
}
Expand Down
11 changes: 0 additions & 11 deletions internal/repository/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,17 +264,6 @@ var CustomConfigItems = []auconfigapi.ConfigItem{
return err
},
},
{
Key: config.KeyAllowedFileCategories,
EnvName: config.KeyAllowedFileCategories,
Default: "",
Description: "allowed filecategory keys",
Validate: func(key string) error {
value := auconfigenv.Get(key)
_, err := parseAllowedFileCategories(value)
return err
},
},
{
Key: config.KeyRedisUrl,
EnvName: config.KeyRedisUrl,
Expand Down
15 changes: 0 additions & 15 deletions internal/repository/config/plumbing.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ type CustomConfigImpl struct {
VRepositoryTypes string
VRepositoryKeySeparator string
VNotificationConsumerConfigs map[string]config.NotificationConsumerConfig
VAllowedFileCategories []string
VRedisUrl string
VRedisPassword string
VPullRequestBuildUrl string
Expand Down Expand Up @@ -120,7 +119,6 @@ func (c *CustomConfigImpl) Obtain(getter func(key string) string) {
c.VRepositoryTypes = getter(config.KeyRepositoryTypes)
c.VRepositoryKeySeparator = getter(config.KeyRepositoryKeySeparator)
c.VNotificationConsumerConfigs, _ = parseNotificationConsumerConfigs(getter(config.KeyNotificationConsumerConfigs))
c.VAllowedFileCategories, _ = parseAllowedFileCategories(getter(config.KeyAllowedFileCategories))
c.VRedisUrl = getter(config.KeyRedisUrl)
c.VRedisPassword = getter(config.KeyRedisPassword)
c.VPullRequestBuildUrl = getter(config.KeyPullRequestBuildUrl)
Expand Down Expand Up @@ -232,19 +230,6 @@ func parseNotificationConsumerConfigs(rawJson string) (map[string]config.Notific
return result, nil
}

func parseAllowedFileCategories(rawJson string) ([]string, error) {
result := make([]string, 0)
if rawJson == "" {
return result, nil
}

if err := json.Unmarshal([]byte(rawJson), &result); err != nil {
return nil, err
}

return result, nil
}

type rawVCSConfig struct {
Platform string `json:"platform"`
APIBaseURL string `json:"apiBaseURL"`
Expand Down
2 changes: 1 addition & 1 deletion internal/repository/config/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestValidate_LotsOfErrors(t *testing.T) {
_, err := tstSetupCutAndLogRecorder(t, "invalid-config-values.yaml")

require.NotNil(t, err)
require.Contains(t, err.Error(), "some configuration values failed to validate or parse. There were 26 error(s). See details above")
require.Contains(t, err.Error(), "some configuration values failed to validate or parse. There were 25 error(s). See details above")

actualLog := goauzerolog.RecordedLogForTesting.String()

Expand Down
42 changes: 0 additions & 42 deletions internal/service/repositories/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,21 +168,6 @@ func (s *Impl) GetRepository(ctx context.Context, repoKey string) (openapi.Repos
repositoryDto.Configuration = &repoConfig
}

if err == nil && repositoryDto.Filecategory != nil {
// filter by allowed keys
allowedKeys := s.CustomConfiguration.AllowedFileCategories()
for key, _ := range repositoryDto.Filecategory {
if !sliceContains(allowedKeys, key) {
delete(repositoryDto.Filecategory, key)
}
}

if len(repositoryDto.Filecategory) == 0 {
// drop empty map completely
repositoryDto.Filecategory = nil
}
}

return repositoryDto, err
}

Expand Down Expand Up @@ -275,7 +260,6 @@ func (s *Impl) mapRepoCreateDtoToRepoDto(repositoryCreateDto openapi.RepositoryC
Url: repositoryCreateDto.Url,
Mainline: repositoryCreateDto.Mainline,
Configuration: repositoryCreateDto.Configuration,
Filecategory: repositoryCreateDto.Filecategory,
Generator: repositoryCreateDto.Generator,
Unittest: repositoryCreateDto.Unittest,
Labels: repositoryCreateDto.Labels,
Expand All @@ -292,9 +276,6 @@ func (s *Impl) validateRepositoryCreateDto(ctx context.Context, key string, dto
if dto.JiraIssue == "" {
messages = append(messages, "field jiraIssue is mandatory")
}
if dto.Filecategory != nil {
messages = s.validateFilecategory(messages, dto.Filecategory)
}

if len(messages) > 0 {
details := strings.Join(messages, ", ")
Expand Down Expand Up @@ -361,9 +342,6 @@ func (s *Impl) validateExistingRepositoryDto(ctx context.Context, key string, dt
if dto.JiraIssue == "" {
messages = append(messages, "field jiraIssue is mandatory for updates")
}
if dto.Filecategory != nil {
messages = s.validateFilecategory(messages, dto.Filecategory)
}

if len(messages) > 0 {
details := strings.Join(messages, ", ")
Expand Down Expand Up @@ -428,9 +406,6 @@ func (s *Impl) validateRepositoryPatchDto(ctx context.Context, key string, patch
messages = validateOwner(messages, dto.Owner)
messages = validateUrl(messages, dto.Url)
messages = validateMainline(messages, dto.Mainline)
if dto.Filecategory != nil {
messages = s.validateFilecategory(messages, dto.Filecategory)
}

if patchDto.CommitHash == "" {
messages = append(messages, "field commitHash is mandatory for patching")
Expand Down Expand Up @@ -458,7 +433,6 @@ func patchRepository(current openapi.RepositoryDto, patch openapi.RepositoryPatc
Generator: patchStringPtr(patch.Generator, current.Generator),
Unittest: patchPtr[bool](patch.Unittest, current.Unittest),
Configuration: patchConfiguration(patch.Configuration, current.Configuration),
Filecategory: patchFilecategory(patch.Filecategory, current.Filecategory),
Labels: patchLabels(patch.Labels, current.Labels),
TimeStamp: patch.TimeStamp,
CommitHash: patch.CommitHash,
Expand Down Expand Up @@ -498,10 +472,6 @@ func patchApprovers(patch map[string][]string, original map[string][]string) map
return patchMapStringListString(patch, original)
}

func patchFilecategory(patch map[string][]string, original map[string][]string) map[string][]string {
return patchMapStringListString(patch, original)
}

func patchMapStringListString(patch map[string][]string, original map[string][]string) map[string][]string {
if patch != nil {
if len(patch) == 0 {
Expand Down Expand Up @@ -700,18 +670,6 @@ func validateMainline(messages []string, mainline string) []string {
return messages
}

func (s *Impl) validateFilecategory(messages []string, filecategories map[string][]string) []string {
allowedCategories := s.CustomConfiguration.AllowedFileCategories()

for category, _ := range filecategories {
if !sliceContains(allowedCategories, category) {
messages = append(messages, fmt.Sprintf("filecategory keys must be one of %s", strings.Join(allowedCategories, ",")))
}
}

return messages
}

func (s *Impl) expandRefProtectionsExemptionLists(ctx context.Context, protections *openapi.RefProtections) *openapi.RefProtections {
if protections == nil {
return protections
Expand Down
29 changes: 12 additions & 17 deletions internal/service/repositories/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func createRepositoryDto() openapi.RepositoryDto {
Generator: ptr("generator"),
Unittest: ptr(true),
Configuration: createRepositoryConfigurationDto(),
Filecategory: map[string][]string{"a": {"path/a.yaml"}},
Labels: map[string]string{"label": "originalValue"},
TimeStamp: "ts",
CommitHash: "hash",
Expand Down Expand Up @@ -155,10 +154,9 @@ func TestPatchRepository_ReplaceAll(t *testing.T) {
Approvers: map[string][]string{"group": {"newapprover1"}},
Archived: ptr(true),
},
Filecategory: map[string][]string{"b": {"b.yaml", "b.json"}},
Labels: map[string]string{"label": "patchedValue"},
TimeStamp: "newts",
CommitHash: "newhash",
Labels: map[string]string{"label": "patchedValue"},
TimeStamp: "newts",
CommitHash: "newhash",
}, openapi.RepositoryDto{
Owner: "newowner",
Url: "newurl",
Expand Down Expand Up @@ -191,10 +189,9 @@ func TestPatchRepository_ReplaceAll(t *testing.T) {
Approvers: map[string][]string{"group": {"newapprover1"}},
Archived: ptr(true),
},
Filecategory: map[string][]string{"b": {"b.yaml", "b.json"}},
Labels: map[string]string{"label": "patchedValue"},
TimeStamp: "newts",
CommitHash: "newhash",
Labels: map[string]string{"label": "patchedValue"},
TimeStamp: "newts",
CommitHash: "newhash",
})
}

Expand All @@ -214,10 +211,9 @@ func TestPatchRepository_ClearFields(t *testing.T) {
},
Approvers: map[string][]string{},
},
Filecategory: map[string][]string{},
Labels: map[string]string{},
TimeStamp: "",
CommitHash: "",
Labels: map[string]string{},
TimeStamp: "",
CommitHash: "",
}, openapi.RepositoryDto{
Owner: "",
Url: "",
Expand All @@ -236,10 +232,9 @@ func TestPatchRepository_ClearFields(t *testing.T) {
Approvers: nil,
Archived: ptr(false),
},
Filecategory: nil,
Labels: nil,
TimeStamp: "",
CommitHash: "",
Labels: nil,
TimeStamp: "",
CommitHash: "",
})
}

Expand Down
2 changes: 0 additions & 2 deletions local-config.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ ALERT_TARGET_REGEX: '(^https://domain[.]com/)|(@domain[.]com$)'

OWNER_ALIAS_FILTER_REGEX: '.*'

ALLOWED_FILE_CATEGORIES: '["template"]'

# The NOTIFICATION_CONSUMER_CONFIGS env below is an example:

#NOTIFICATION_CONSUMER_CONFIGS: >-
Expand Down
14 changes: 3 additions & 11 deletions test/acceptance/util_dtos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,6 @@ func tstUpdatedServicePayload(name string) openapi.NotificationPayload {
// repository

func tstRepository() openapi.RepositoryDto {
fc := map[string][]string{
"cached-template": {"cached-templates/tpl1.yaml", "more/cached/templates/tpl2.yaml"},
}
return openapi.RepositoryDto{
Owner: "some-owner",
Url: "ssh://git@bitbucket.some-organisation.com:7999/helm/karma-wrapper.git",
Expand Down Expand Up @@ -259,10 +256,9 @@ func tstRepository() openapi.RepositoryDto {
},
Approvers: map[string][]string{"testing": {"some-user"}},
},
Filecategory: fc,
TimeStamp: "2022-11-06T18:14:10Z",
CommitHash: "6c8ac2c35791edf9979623c717a243fc53400000",
JiraIssue: "ISSUE-2345",
TimeStamp: "2022-11-06T18:14:10Z",
CommitHash: "6c8ac2c35791edf9979623c717a243fc53400000",
JiraIssue: "ISSUE-2345",
}
}

Expand Down Expand Up @@ -353,10 +349,6 @@ configuration:
requireConditions:
snyk-key:
refMatcher: master
filecategory:
cached-template:
- cached-templates/tpl1.yaml
- more/cached/templates/tpl2.yaml
`
}

Expand Down
3 changes: 0 additions & 3 deletions test/mock/metadatamock/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,6 @@ configuration:
const deployment2 = `mainline: main
url: ssh://git@bitbucket.some-organisation.com:7999/PROJECT/whatever-deployment.git
generator: third-party-software
filecategory:
forbidden-key:
- some/interesting/file.txt
`

const implementation = `mainline: master
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@
]
}
},
"filecategory": {
"cached-template": [
"cached-templates/tpl1.yaml",
"more/cached/templates/tpl2.yaml"
]
},
"jiraIssue": "ISSUE-2345",
"mainline": "master",
"owner": "some-owner",
Expand Down
Loading

0 comments on commit 844e03e

Please sign in to comment.