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 overriding build network from properties #1636

Merged
merged 7 commits into from
Feb 5, 2023
Merged

Allow overriding build network from properties #1636

merged 7 commits into from
Feb 5, 2023

Conversation

ktulinger
Copy link
Contributor

@ktulinger ktulinger commented Jan 29, 2023

The configuration of

<images>
  <image>
      <name>xx/%a:%l</name>
      <alias>xxxx</alias>
      <build>
          <dockerFile>${project.basedir}/Dockerfile</dockerFile>
          <network>host</network>
      </build>

      <external>
          <type>properties</type>
          <prefix>docker</prefix>
          <mode>override</mode>
      </external>
  </image>
</images>

simply does not propagate the network option further. When omitting "external" it works as expected. The network field is not resolved when external configuration resolution takes place.

Network field was introduced in #1288

@ktulinger
Copy link
Contributor Author

I think I filled the documentation too, please check once you have time

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #1636 (d6ea9e5) into master (01c0b85) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1636      +/-   ##
============================================
+ Coverage     63.76%   63.78%   +0.02%     
  Complexity     2177     2177              
============================================
  Files           170      170              
  Lines          9926     9928       +2     
  Branches       1361     1361              
============================================
+ Hits           6329     6333       +4     
+ Misses         3064     3062       -2     
  Partials        533      533              
Impacted Files Coverage Δ
...aven/docker/config/handler/property/ConfigKey.java 100.00% <100.00%> (ø)
...config/handler/property/PropertyConfigHandler.java 85.95% <100.00%> (+0.04%) ⬆️
...8/maven/docker/config/BuildImageConfiguration.java 85.09% <0.00%> (+0.78%) ⬆️

@rohanKanojia
Copy link
Member

@ktulinger : Thanks, could you please add a line to changelog and a test if possible?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@rohanKanojia rohanKanojia merged commit e816499 into fabric8io:master Feb 5, 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 this pull request may close these issues.

2 participants