Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow custom user renovate configs, in config maps, #372

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rcerven
Copy link
Contributor

@rcerven rcerven commented Nov 13, 2024

there can be one global one for all components, and also custom config for specific components

we are also now creating renovate config in json format

STONEBLD-2916

Checklist:

  • PR has reference to the issue(s) it resolves
  • Write / update unit tests
  • Write / update integration (envtest) tests
  • Ensure there is an issue for e2e tests if needed
  • Ensure make test passes
  • Ensure test coverage hasn't decreased
  • Test code changes manually
  • Update readme and documentation
  • Write PR description that highlights overall changes and add usage examples if applicable

@rcerven rcerven changed the title allow custom user renovate configs, in config maps, [WIP] allow custom user renovate configs, in config maps, Nov 13, 2024
there can be one global one for all components, and also
custom config for specific components

we are also now creating renovate config in json format

STONEBLD-2916

Signed-off-by: Robert Cerven <[email protected]>
@rcerven rcerven changed the title [WIP] allow custom user renovate configs, in config maps, allow custom user renovate configs, in config maps, Nov 14, 2024
Comment on lines 394 to 396
randomStr1 := RandomString(5)
randomStr2 := RandomString(10)
randomStr3 := RandomString(10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use more descriptive names for these variables?

Comment on lines +412 to +422
if globalConfigString != "" || componentConfigString != "" {
if componentConfigString != "" {
configString = componentConfigString
configType = componentConfigType
log.Info("will use custom renovate config for component", "name", componentConfigName, "type", configType)
} else {
configString = globalConfigString
configType = globalConfigType
log.Info("will use custom global renovate config", "name", GlobalRenovateConfigName, "type", configType)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if globalConfigString != "" || componentConfigString != "" {
if componentConfigString != "" {
configString = componentConfigString
configType = componentConfigType
log.Info("will use custom renovate config for component", "name", componentConfigName, "type", configType)
} else {
configString = globalConfigString
configType = globalConfigType
log.Info("will use custom global renovate config", "name", GlobalRenovateConfigName, "type", configType)
}
if componentConfigString != "" {
configString = componentConfigString
configType = componentConfigType
log.Info("will use custom renovate config for component", "name", componentConfigName, "type", configType)
} else if globalConfigString != "" {
configString = globalConfigString
configType = globalConfigType
log.Info("will use custom global renovate config", "name", GlobalRenovateConfigName, "type", configType)

matchStrings = append(matchStrings, buildResult.BuiltImageRepository+"(:.*)?@(?<currentDigest>sha256:[a-f0-9]+)")
matchPackageNames = append(matchPackageNames, buildResult.BuiltImageRepository)

for _, drepositiry := range buildResult.DistributionRepositories {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: drepositiry -> drepository

@@ -245,6 +240,8 @@ var _ = Describe("Component nudge controller", func() {

renovatePipelines := getRenovatePipelineRunList()
Expect(len(renovatePipelines)).Should(Equal(1))
renovateCommand := strings.Join(renovatePipelines[0].Spec.PipelineSpec.Tasks[0].TaskSpec.Steps[0].Command, ";")
Expect(strings.Contains(renovateCommand, `'username':'image_repo_username'`)).Should(BeTrue())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi there are Gomega matchers like HaveLen() and replacements for strings like ContainSubstring() and HaveSuffix() which make the Expects a bit easier to read

Expect(renovateCommand).To(ContainSubstring(\'username':'image_repo_username'\`))

Comment on lines +457 to +465
customConfigName := fmt.Sprintf("nudging-renovate-config-%s", Operator1)
customConfigMapName1 := types.NamespacedName{Namespace: UserNamespace, Name: customConfigName}
customConfigString := `{"username":"componentconfiguserjs"}`
customConfigMapData := map[string]string{ConfigKeyJs: customConfigString}
createCustomRenovateConfigMap(customConfigMapName1, customConfigMapData)
customConfigName = fmt.Sprintf("nudging-renovate-config-%s", Operator2)
customConfigMapName2 := types.NamespacedName{Namespace: UserNamespace, Name: customConfigName}
createCustomRenovateConfigMap(customConfigMapName2, customConfigMapData)
customConfigType := "js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test setups are a bit hard to read. Could you separate the ConfigMaps by linebreaks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants