You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/04/05 14:14:14 UTC
[GitHub] [camel-k] johnpoth opened a new pull request, #3171: fix #2835 address comments
johnpoth opened a new pull request, #3171:
URL: https://github.com/apache/camel-k/pull/3171
<!-- Description -->
Addresses comments by @astefanutti in https://github.com/apache/camel-k/pull/2835
Thanks !
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] johnpoth commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
johnpoth commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843725993
##########
pkg/builder/project.go:
##########
@@ -91,6 +94,16 @@ func generateJavaKeystore(ctx *builderContext) error {
return jvm.GenerateKeystore(ctx.C, ctx.Path, ctx.Maven.TrustStoreName, ctx.Maven.TrustStorePass, certsData)
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
Hi @tadayosi ! Thanks for reviewing!
I didn't know a slice cannot be nil in Go ?
https://go.dev/tour/moretypes/12#:~:text=The%20zero%20value%20of%20a,and%20has%20no%20underlying%20array.
I just wrote it to be bullet proof but I am happy to remove it if you think it's unnecessary ?
Thanks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843430334
##########
pkg/util/camel/catalog.go:
##########
@@ -94,8 +95,10 @@ func GenerateCatalog(
}
var caCerts [][]byte
- if mvn.CASecret != nil {
- caCerts, err = kubernetes.GetSecretsRefData(ctx, client, namespace, mvn.CASecret)
+ // nolint: staticcheck
+ secrets := mergeSecrets(mvn.CASecrets, mvn.CASecret)
+ if secrets != nil {
Review Comment:
same here
##########
pkg/util/camel/catalog.go:
##########
@@ -104,6 +107,16 @@ func GenerateCatalog(
return GenerateCatalogCommon(ctx, globalSettings, []byte(userSettings), caCerts, mvn, runtime, providerDependencies)
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843430039
##########
pkg/trait/openapi.go:
##########
@@ -272,9 +272,11 @@ func (t *openAPITrait) createNewOpenAPIConfigMap(e *Environment, resource v1.Dat
return err
}
mc.GlobalSettings = data
+ // nolint: staticcheck
+ secrets := mergeSecrets(e.Platform.Status.Build.Maven.CASecrets, e.Platform.Status.Build.Maven.CASecret)
- if e.Platform.Status.Build.Maven.CASecret != nil {
- certsData, err := kubernetes.GetSecretsRefData(e.Ctx, e.Client, e.Platform.Namespace, e.Platform.Status.Build.Maven.CASecret)
+ if secrets != nil {
Review Comment:
same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] johnpoth commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
johnpoth commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r846032007
##########
pkg/builder/project.go:
##########
@@ -91,6 +94,16 @@ func generateJavaKeystore(ctx *builderContext) error {
return jvm.GenerateKeystore(ctx.C, ctx.Path, ctx.Maven.TrustStoreName, ctx.Maven.TrustStorePass, certsData)
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
Let me merge this for now and we can reiterate later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843429641
##########
pkg/builder/project.go:
##########
@@ -91,6 +94,16 @@ func generateJavaKeystore(ctx *builderContext) error {
return jvm.GenerateKeystore(ctx.C, ctx.Path, ctx.Maven.TrustStoreName, ctx.Maven.TrustStorePass, certsData)
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
Is this check necessary? `secrets == nil`
I think it's always safe to assume a slice is not nil in Go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] johnpoth merged pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
johnpoth merged PR #3171:
URL: https://github.com/apache/camel-k/pull/3171
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] johnpoth commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
johnpoth commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843758722
##########
pkg/builder/project.go:
##########
@@ -91,6 +94,16 @@ func generateJavaKeystore(ctx *builderContext) error {
return jvm.GenerateKeystore(ctx.C, ctx.Path, ctx.Maven.TrustStoreName, ctx.Maven.TrustStorePass, certsData)
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
After running it debug mode it seems that `ctx.Build.Maven.CASecrets` does get set to nil so the check is necessary ? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3171: fix #2835 address comments
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3171:
URL: https://github.com/apache/camel-k/pull/3171#discussion_r843430146
##########
pkg/trait/openapi.go:
##########
@@ -348,6 +350,16 @@ func (t *openAPITrait) createNewOpenAPIConfigMap(e *Environment, resource v1.Dat
return nil
}
+func mergeSecrets(secrets []corev1.SecretKeySelector, secret *corev1.SecretKeySelector) []corev1.SecretKeySelector {
+ if secrets == nil && secret == nil {
Review Comment:
same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org