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

Config to specify isolation technology for container #1376

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

rahul-kulkarni73
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Sep 19, 2020

Codecov Report

Merging #1376 (1ad3cad) into master (3d9cdbf) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1376      +/-   ##
============================================
+ Coverage     59.03%   59.07%   +0.04%     
- Complexity     1985     1989       +4     
============================================
  Files           162      162              
  Lines          9023     9032       +9     
  Branches       1364     1365       +1     
============================================
+ Hits           5327     5336       +9     
  Misses         3205     3205              
  Partials        491      491              
Impacted Files Coverage Δ Complexity Δ
...bric8/maven/docker/access/ContainerHostConfig.java 93.20% <100.00%> (+0.06%) 50.00 <1.00> (+1.00)
...ic8/maven/docker/config/RunImageConfiguration.java 91.66% <100.00%> (+0.15%) 54.00 <1.00> (+1.00)
...ig/handler/compose/DockerComposeConfigHandler.java 71.87% <100.00%> (+0.29%) 17.00 <0.00> (ø)
...g/handler/compose/DockerComposeServiceWrapper.java 61.76% <100.00%> (+0.18%) 63.00 <1.00> (+1.00)
...aven/docker/config/handler/property/ConfigKey.java 100.00% <100.00%> (ø) 10.00 <0.00> (ø)
...config/handler/property/PropertyConfigHandler.java 80.91% <100.00%> (+0.07%) 136.00 <0.00> (+1.00)
...va/io/fabric8/maven/docker/service/RunService.java 62.63% <100.00%> (+0.13%) 40.00 <0.00> (ø)

@dermot-hardy
Copy link

Hi. Is there a way to reference the plugin built from this branch?

@rohanKanojia
Copy link
Member

I think https://jitpack.io/ might help

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR ! Sound reasonable to me, we will consider this for one of the next releases (but please note that this plugin is on low maintenance mode, which means things might take a bit longer than normally).

Do you know since which API version the Isolation option is available for Docker ?

@rahul-kulkarni73
Copy link
Contributor Author

Kernel namespaces were introduced between kernel version 2.6.15 and 2.6.26. This means that since July 2008 (date of the 2.6.26 release ), namespace code has been exercised and scrutinized on a large number of production systems.
https://man7.org/linux/man-pages/man7/namespaces.7.html

Seems like docker allowed process isolation in this version
https://github.com/docker/docker-ce/releases/tag/v18.09.1 (Jan 2019)
docker-archive/engine#81

Found this docker doc dated 2016 https://www.docker.com/sites/default/files/WP_IntrotoContainerSecurity_08.19.2016.pdf.

@sonarcloud
Copy link

sonarcloud bot commented Sep 28, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@rahul-kulkarni73
Copy link
Contributor Author

@rhuss Any updates on possibility of getting this change in ?

@rohanKanojia
Copy link
Member

Sorry for late response. I think we should get this in for our upcoming release (0.35.0) Let's try to get this merged in coming week so that we can have it in our next release

@rohanKanojia rohanKanojia self-requested a review March 7, 2021 06:02
@rohanKanojia rohanKanojia added this to the 0.35.0 milestone Mar 7, 2021
Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and I guess that since this feature is added on request, it's not harmful when running against an older Docker that is not supporting that parameter (as in this case the user needs to take care whether to configure Isolation or not)

Please add a doc/changelog.md entry, then I think we are good to merge.

@rohanKanojia rohanKanojia merged commit b0668e4 into fabric8io:master Mar 12, 2021
@dermot-hardy dermot-hardy deleted the isolationTech branch March 12, 2021 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants