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

Jib not using image name when using variables #33552

Closed
cescoffier opened this issue May 23, 2023 · 16 comments · Fixed by #33560
Closed

Jib not using image name when using variables #33552

cescoffier opened this issue May 23, 2023 · 16 comments · Fixed by #33560
Milestone

Comments

@cescoffier
Copy link
Member

cescoffier commented May 23, 2023

Describe the bug

If I use Jib to build a container with the following property (voluntarily without value):

quarkus.container-image.name=hide-and-seek-${application.name}

It ignores the value and uses the artifactId as the default image name.

If I use:

quarkus.container-image.name=hide-and-seek-place-service

It works.

Expected behavior

It should use the set image name, at least try to resolve it, and fails, or at least report something instead of switching silently to the default name.

Actual behavior

It does not.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@cescoffier cescoffier added the kind/bug Something isn't working label May 23, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented May 23, 2023

/cc @geoand (jib)

@geoand
Copy link
Contributor

geoand commented May 23, 2023

@cescoffier that's the expected behavior because application.name is not a property that exists.

If you use quarkus.container-image.name=hide-and-seek-${quarkus.application.name} it should work. Can you try?

@cescoffier
Copy link
Member Author

Yes, that's my point; it should NOT go back to the default without telling anything. Either fail (you're dumb - that property does not exist) or replace it using an empty string.

@cescoffier
Copy link
Member Author

(I would prefer a failure - as mandatory properties are ... mandatory)

@geoand
Copy link
Contributor

geoand commented May 23, 2023

That's how Smallrye Config works ATM. If we want to change the expected behavior, we would need to make the change there - we would need @radcortez for this.

@cescoffier
Copy link
Member Author

Yes, I agree. At least having a BIG warning telling me that something is wrong and I may do something really wrong (which was the case, as now I've lots of images in my docker registry)

@cescoffier
Copy link
Member Author

cescoffier commented May 23, 2023

BTW, why is this log printed twice?

[INFO] [io.quarkus.container.image.jib.deployment.JibProcessor] Container entrypoint set to [java, -Djava.util.logging.manager=org.jboss.logmanager.LogManager, -jar, quarkus-run.jar]
[INFO] [io.quarkus.container.image.jib.deployment.JibProcessor] Container entrypoint set to [java, -Djava.util.logging.manager=org.jboss.logmanager.LogManager, -jar, quarkus-run.jar]

@geoand
Copy link
Contributor

geoand commented May 23, 2023

Dunno, I am not seeing this

@radcortez
Copy link
Member

The issue is not with SmallRyeConfig. It is how is mapped.

If you map it as a String and if an expansion value does not exist, you get a NoSuchElementException. If you map it as an Optional<String> and if an expansion value does not exist, you get an empty Optional.

The ConfigRoot is mapped as an Optional:

And the extension does silently fallback to the application name if there is no value:

String effectiveName = containerImageCustomName.map(ContainerImageCustomNameBuildItem::getName)
.or(() -> containerImageConfig.name)
.orElse(app.getName());

@geoand
Copy link
Contributor

geoand commented May 23, 2023

Thanks @radcortez for the analysis!

I'll have a closer look.

@radcortez
Copy link
Member

BTW, since the @ConfigRoot sets a default:

@ConfigItem(defaultValue = "${quarkus.application.name:unset}")
@ConvertWith(TrimmedStringConverter.class)
public Optional<String> name;

There is not much value in mapping this with an Optional, unless the configuration is explicitly emptied in the configuration with quarkus.container-image.name=.

@geoand
Copy link
Contributor

geoand commented May 23, 2023

So if we did:

 @ConfigItem(defaultValue = "${quarkus.application.name:unset}") 
 @ConvertWith(TrimmedStringConverter.class) 
 public String name; 

the expected behavior would be for quarkus.container-image.name=hide-and-seek-${application.name} to fail?

@radcortez
Copy link
Member

Yes, you should get a java.util.NoSuchElementException: SRCFG00011: Could not expand value application.name in property quarkus.container-image.name

@geoand
Copy link
Contributor

geoand commented May 23, 2023

Understood, thanks for the help.

I opened #33560

@geoand geoand changed the title Jib not using image name when using vairables Jib not using image name when using variables May 23, 2023
@cescoffier
Copy link
Member Author

Wow! Thanks @radcortez for the explanation! It makes sense! We should document that somewhere, as I'm sure lots of extension author do not see the difference of behavior!

geoand added a commit to geoand/quarkus that referenced this issue May 23, 2023
@geoand
Copy link
Contributor

geoand commented May 23, 2023

+100 for documenting it!

geoand added a commit that referenced this issue May 23, 2023
Don't ignore invalid config in quarkus.container-image.name
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants