Skip to content

builder entitlements configuration added.#39144

Merged
tiborvass merged 1 commit intomoby:masterfrom
kunalkushwaha:builder-entitilement-confg
Sep 30, 2019
Merged

builder entitlements configuration added.#39144
tiborvass merged 1 commit intomoby:masterfrom
kunalkushwaha:builder-entitilement-confg

Conversation

@kunalkushwaha
Copy link
Copy Markdown
Contributor

buildkit supports entitlements like network-host and security-insecure.
this patch aims to make it configurable through daemon.json file.
by default network-host is enabled & secuirty-insecure is disabled.

Signed-off-by: Kunal Kushwaha kunal.kushwaha@gmail.com

- What I did
option to enable/disable buildkit's entitlements settings for moby
fixes part of issue discussed here moby/buildkit#950 (comment)

- How I did it
Added entitlements in builder config

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 26, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@4e9cffa). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #39144   +/-   ##
=========================================
  Coverage          ?   37.01%           
=========================================
  Files             ?      612           
  Lines             ?    45400           
  Branches          ?        0           
=========================================
  Hits              ?    16806           
  Misses            ?    26307           
  Partials          ?     2287

@thaJeztah thaJeztah changed the title builder entitlements configutation added. builder entitlements configuration added. Apr 27, 2019
@kunalkushwaha kunalkushwaha force-pushed the builder-entitilement-confg branch from 8c7aab6 to cba50de Compare May 7, 2019 06:21
@andrewrynhard
Copy link
Copy Markdown

Is it still possible to get this into 19.03?

moby/buildkit#950 (comment)

Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Small nit, overall LGTM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: conf.Entitlements.NetworkHost != nil && not needed here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing conf.Entitlements.NetworkHost != nil && .. will make only way to enable Network-host is by no entry in config file.

{
        "features": {
                "buildkit": true
        },
        "builder":{
                "entitlements":{
                        "network-host":true,
                        "security-insecure":false
                        }
        }
}

The above config file will also not enable network-host. I think that will be wrong behaviour.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I only meant the conf.Entitlements.NetworkHost != nil && part not *conf.Entitlements.NetworkHost == true after it that is still needed. Just as small cleanup because you already check if it is nil before with no functional change.

@tonistiigi
Copy link
Copy Markdown
Member

ping @kunalkushwaha

@kunalkushwaha
Copy link
Copy Markdown
Contributor Author

Sorry somehow, I missed this. I will update it.

@kunalkushwaha kunalkushwaha force-pushed the builder-entitilement-confg branch from cba50de to 1dc8318 Compare August 23, 2019 02:47
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

looks like there's two minor linting issues, causing CI to fail; could you address those? Thanks!

@kunalkushwaha kunalkushwaha force-pushed the builder-entitilement-confg branch 2 times, most recently from c705743 to e180ea5 Compare September 4, 2019 00:39
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kunalkushwaha remove debug

@kunalkushwaha kunalkushwaha force-pushed the builder-entitilement-confg branch from e180ea5 to 4a21d0e Compare September 6, 2019 02:35
buildkit supports entitlements like network-host and security-insecure.
this patch aims to make it configurable through daemon.json file.
by default network-host is enabled & secuirty-insecure is disabled.

Signed-off-by: Kunal Kushwaha <kunal.kushwaha@gmail.com>
@kunalkushwaha kunalkushwaha force-pushed the builder-entitilement-confg branch from 4a21d0e to 8b7bbf1 Compare September 26, 2019 08:05
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.

7 participants