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

Gradle: Property "container.labels" does not respect "convention" values #3986

Closed
ChristianCiach opened this issue Apr 20, 2023 · 6 comments
Closed

Comments

@ChristianCiach
Copy link
Contributor

ChristianCiach commented Apr 20, 2023

Environment:

  • Jib version: 3.3.1
  • Build tool: Gradle 8.1
  • OS: Fedora Workstation 37

Description of the issue:

I am trying to set convention-values to the Gradle MapProperty jib.container.labels from within my Gradle Init-Script. Unfortunately, this doesn't work, because the MapProperty is explicitly initialized with .empty() by the Jib plugin, overriding any convention values of property.

labels = objectFactory.mapProperty(String.class, String.class).empty();

I've prepared a commit to fix this, but the PR-template advises me to create an issue first. My fix: ChristianCiach@7b6f5b5

Expected behavior:

Convention-values should be respected.

Steps to reproduce:

  1. Set jib.container.labels.convention(['a': 'b']) in your gradle script.
  2. Do not explicitly set jib.container.labels to any value.
  3. No labels in generated image :(

jib-gradle-plugin Configuration:

jib {
    configurationName = SpringBootPlugin.PRODUCTION_RUNTIME_CLASSPATH_CONFIGURATION_NAME
    from {
        image = 'company-internal-image:latest'
    }
    container {
        mainClass = project.mainClassName
        ports = ['8080']
        creationTime = 'USE_CURRENT_TIMESTAMP'
    }
}

Relevant part of my init-script:

void configureJib(Project p) {
    p.pluginManager.withPlugin('com.google.cloud.tools.jib') { jibPlugin ->
        def defaultLabels = getConventionContainerLabels()

        // "jib" is the name of the project extension applied by the jib plugin.
        // See: com.google.cloud.tools.jib.gradle.JibPlugin.JIB_EXTENSION_NAME
        p.extensions.configure("jib") { jib ->
            jib.container.labels.convention(defaultLabels)
        }
    }
}
@ChristianCiach ChristianCiach changed the title Gradle: Property "container.labels" should not explicitly be set to empty Gradle: Property "container.labels" does not respect "convention" values Apr 20, 2023
@ChristianCiach
Copy link
Contributor Author

I just notice that the class ContainerParameters is very inconsistent about its return values. getLabels() is the only Method in that class that directly returns the underlying MapProperty. The other @Input methods do not expose the gradle properties, like String getMainClass() and List<String> getJvmFlags(). That's a bummer, because I actually do want to set convention-values for jvmFlags from within my gradle-init-script, but the getter method does not expose the underlying ListProperty, so I cannot call convention(..) on it.

@ChristianCiach
Copy link
Contributor Author

Actually, I think Jib should not try to hide the ListProperty and MapProperty input-variables from the users. getLabels() has successfully been converted from Map to MapProperty in the past. I don't know if this is a breaking change, but I think the other @Input-methods should also directly return the underlying *Property objects. This is how the Gradle documentation says it should be done.

@chanseokoh
Copy link
Member

Yeah, when we wrote the code for the input variables, we were not so familiar with Gradle. I still don't know what the difference between calling .convention() or .empty() is for the purpose of setting a default value. Some properties have been converted to Property and exposed recently (mostly external contributions), but most of them are still legacy code.

Some of the past issues (not an exhaustive list): #3927 #3094 #3489 #3708 #2727

@diegomarquezp
Copy link
Contributor

Hi @ChristianCiach, thanks for bringing this up. Since we are accepting contributions, the best way to determine if your suggestion is non-breaking is by running tests on a PR, so feel free to go ahead with it!

cc'ing @emmileaf and @mpeddada1 - you may have more info on this

@blakeli0
Copy link
Contributor

blakeli0 commented Mar 5, 2024

Convention values are deprecated in Gradle 8.6, closing as not planned.

@blakeli0 blakeli0 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2024
@ChristianCiach
Copy link
Contributor Author

@blakeli0 The deprecated Convention interface that you linked has absolutely nothing to do with the Property.convention(..) values that this issue is all about. See https://docs.gradle.org/current/javadoc/org/gradle/api/provider/Property.html#convention-T-

These are not deprecated and this issue is still valid.

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

No branches or pull requests

5 participants