Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: make security context configs more restrictive #3454

Merged
merged 1 commit into from Jun 12, 2020

Conversation

vtikoo
Copy link
Contributor

@vtikoo vtikoo commented Jun 11, 2020

Reason for Change:
Updated the security context related configs of the sgx device plugin to be more restrictive.

Issue Fixed:

Requirements:

Notes:

@acs-bot acs-bot added the size/S label Jun 11, 2020
@vtikoo
Copy link
Contributor Author

vtikoo commented Jun 11, 2020

/assign @salsal97 @jackfrancis

Copy link
Contributor

@salsal97 salsal97 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! Does the test also need to be updated? Its at test/e2e/kubernetes/workloads/sgx-test.yaml

@jackfrancis jackfrancis added this to In progress in backlog Jun 11, 2020
@mboersma
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jackfrancis
Copy link
Member

We should definitely keep the E2E test up-to-date w/ doc recommendations!

@vtikoo
Copy link
Contributor Author

vtikoo commented Jun 11, 2020

@salsal97 @jackfrancis the e2e test is a slightly different scenario, as its not using a device plugin and directly accessing /dev/sgx on the host.
We can however update the e2e test to use a device plugin in a future PR, as thats the workflow we are recommending users. Any thoughts on that?

@jackfrancis
Copy link
Member

@vtikoo Makes sense. So it sounds like we're not really testing/validating what we're recommending?

What would be involved to update our tests? Is it as simple as updating the spec we kubectl apply onto the cluster?

@vtikoo
Copy link
Contributor Author

vtikoo commented Jun 12, 2020

@jackfrancis It would involve installing the sgx device plugin before the actual test workload runs.

For installing the device plugin, we could either kubectl apply it and wait for it to be ready, or create an addon/extension. The latter would be more effort, but might be useful to users as well.

@jackfrancis
Copy link
Member

Right, thanks for clarifying, yeah, let's add a follow-up task to do the kubectl apply + run updated spec in the E2E tests.

backlog automation moved this from In progress to Review in progress Jun 12, 2020
Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot acs-bot added the lgtm label Jun 12, 2020
@acs-bot
Copy link

acs-bot commented Jun 12, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, salsal97, vtikoo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jackfrancis
Copy link
Member

Added this backlog issue: #3462

@jackfrancis jackfrancis merged commit d3d7bc8 into Azure:master Jun 12, 2020
backlog automation moved this from Review in progress to Done Jun 12, 2020
@vtikoo vtikoo deleted the vtikoo/fix-sgx-dp-privileges branch June 12, 2020 17:36
penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
Co-authored-by: Vikas Tikoo <vikas.tikoo@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants