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.

Sign up for GitHub

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

Jump to bottom

Using property jkube.enricher.jkube-name.name overrides the names set in the resources fragments #2823

Open
amihalfa opened this issue Mar 15, 2024 · 4 comments · May be fixed by #2825
Open

Using property jkube.enricher.jkube-name.name overrides the names set in the resources fragments #2823

amihalfa opened this issue Mar 15, 2024 · 4 comments · May be fixed by #2825
Assignees
@amihalfa
Labels
bug Something isn't working

Comments

@amihalfa
Copy link

Describe the bug

I saw in documentation that jkube.enricher.jkube-name.name is supposed to override "Add a default name to every object which misses a name."

When I have 2 pods fragments such as :
src/main/jkube/test-health.yml

apiVersion: v1
kind: Pod
metadata:
  namespace: mynamespace
  name: myapp-test-health
spec:
...

and src/main/jkube/test-metrics.yml

apiVersion: v1
kind: Pod
metadata:
  namespace: mynamespace
  name: myapp-test-metrics
spec:
...

And I set this config in my pom.xml

     <plugin>
                <groupId>org.eclipse.jkube</groupId>
                <artifactId>kubernetes-maven-plugin</artifactId>
                <version>${jkube.version}</version>
                <configuration>
                    <enricher>
                        <config>
                            <jkube-name>
                                <name>myapp</name>
                            </jkube-name>
                            <jkube-well-known-labels>
                                <name>myapp</name>
                            </jkube-well-known-labels>
                        </config>

                    </enricher>

                </configuration>
            </plugin>

The result after build is that I get 2 files named : myapp-pod.yaml and myapp-1-pod.yaml having exactly the same name (set in the pom.xml) :

apiVersion: v1
kind: Pod
metadata:
  namespace: mynamespace
  name: myapp
spec:
...

Eclipse JKube version

1.16.1

Component

Kubernetes Maven Plugin

Apache Maven version

3.8.5

Gradle version

None

Steps to reproduce

  1. Create a pod fragment files with a name :
apiVersion: v1
kind: Pod
metadata:
  namespace: mynamespace
  name: myapp-test
spec:
...
  1. Override the property jkube.enricher.jkube-name.name in the pom.xml :
                    <enricher>
                        <config>
                            <jkube-name>
                                <name>myapp</name>
                            </jkube-name>
                        </config>
                    </enricher>
  1. Build the project using : ./mvnw k8s:resource k8s:helm
  2. Check in your target the generated yaml file myapp-pod.yaml, you will see that the name was changed :
apiVersion: v1
kind: Pod
metadata:
  namespace: mynamespace
  name: myapp
spec:
...

Expected behavior

If a name is set in the fragment, keep the name, if not, override it.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.25.3

Environment

macOS

Eclipse JKube Logs

No response

Sample Reproducer Project

No response

Additional context

No response
@amihalfa amihalfa added the bug Something isn't working label Mar 15, 2024
@rohanKanojia
Copy link
Member

@amihalfa : Is it possible for you to debug this to check what's wrong with Enricher behavior?

  1. Set a breakpoint in Enricher
  2. Run mvnDebug k8s:resource from terminal
  3. Connect Via Remote JVM Debug from your IDE

I see that in code we're only setting name when it's blank:
https://github.com/eclipse/jkube/blob/c4ee9d4e9432297c50d9385a8c06d1fea0f75e3a/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/NameEnricher.java#L57-L60

@amihalfa
Copy link
Author

amihalfa commented Mar 15, 2024 •
edited
Loading

Hello @rohanKanojia

When I debug :

  • configuredName has the value set in my pom.xml
  • defaultName has the value of my ${project.artifactId}

The resource name is always overwritten if a configuredName is set. From my point of view, if the resource already has a name, we keep the name, even if there is a configuredName.

My expected behaviour is to have in NameEnricher something like this :

        builder.accept(new TypedVisitor<ObjectMetaBuilder>() {
            @Override
            public void visit(ObjectMetaBuilder resource) {
                if (StringUtils.isBlank(resource.getName())) {
                    if (StringUtils.isNotBlank(configuredName)) {
                        resource.withName(configuredName);
                    } else {
                        resource.withName(defaultName);
                    }
                }
            }
        });

Thanks !

@rohanKanojia
Copy link
Member

@amihalfa : Your approach looks correct to me. Is it possible to contribute a pull request for this?

@amihalfa amihalfa linked a pull request Mar 15, 2024 that will close this issue
Open
4 tasks
@manusa
Copy link
Member

manusa commented Mar 25, 2024

We need to further discuss this to set the basic scenarios and user expectations.

The suggested approach breaks the existing functionality to override a name via system property or CLI flag.

The current approach is that regardless of what you have in the static resources (such as fragments), you can always perform an override in the command-line by providing a -Djkube.enricher.jkube-name.name=overridden.

If we want to change this, we'll need to properly document it and also provide specific examples and scenarios for users:

  • A name is provided via fragment
  • A name is provided via configuration
  • A name is provided via fragment and configuration
  • Precedence definition
  • How to override behavior in each case (property in fragment vs. flag override precedence, etc.)

@rohanKanojia rohanKanojia moved this to Planned in Eclipse JKube Apr 3, 2024
@rohanKanojia rohanKanojia moved this from Planned to In Progress in Eclipse JKube Apr 25, 2024
@rohanKanojia rohanKanojia moved this from In Progress to Planned in Eclipse JKube May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@amihalfa amihalfa

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check if the fragment doesnot already have a name amihalfa/jkube
3 participants
@manusa @rohanKanojia @amihalfa