Skip to content

api bump#20152

Merged
deads2k merged 6 commits intoopenshift:masterfrom
bparees:apibump
Jun 29, 2018
Merged

api bump#20152
deads2k merged 6 commits intoopenshift:masterfrom
bparees:apibump

Conversation

@bparees
Copy link
Contributor

@bparees bparees commented Jun 29, 2018

No description provided.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2018
Jan Wozniak and others added 6 commits June 29, 2018 09:25
…ions for controlling no-new-privs flag.

This commit adds two new members to SecurityContextConstraints:

* AllowPrivilegeEscalation -- controls whether a container may request privilege escalation or not
  (by setting SecurityContext.AllowPrivilegeEscalation: true). When SCC doesn't define this member,
  it defaulted to "true" for backward/Kubernetes compatibility reasons.

* DefaultAllowPrivilegeEscalation -- sets default value for container's
  SecurityContext.AllowPrivilegeEscalation field when it wasn't explicitly specified.
@bparees
Copy link
Contributor Author

bparees commented Jun 29, 2018

@php-coder @wozniakjan here's my attempt at this. we'll see which one makes it.

# openshift second
- package: github.com/openshift/api
version: 0ce1df2db7debb15eddb25f3ae76df4180777221
version: master
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@deads2k
Copy link
Contributor

deads2k commented Jun 29, 2018

Are the original pulls for these already lgtm'd?

@openshift/sig-security

@liggitt
Copy link
Contributor

liggitt commented Jun 29, 2018

glide, api, and generation changes LGTM. defer to build and security teams for the validation and use of the fields

@simo5
Copy link
Contributor

simo5 commented Jun 29, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 29, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, simo5

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

The pull request process is described here

Details 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

@deads2k deads2k added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 29, 2018
@deads2k
Copy link
Contributor

deads2k commented Jun 29, 2018

This is going to make the re-rebase to 1.11 GA much easier. bumping queue priority.

@bparees bparees mentioned this pull request Jun 29, 2018
@deads2k
Copy link
Contributor

deads2k commented Jun 29, 2018

#20157
/retest

@deads2k
Copy link
Contributor

deads2k commented Jun 29, 2018

#20157 again

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@deads2k
Copy link
Contributor

deads2k commented Jun 29, 2018

this is needed for the re-rebase to GA. conformance install was fine. The base level is really close. Nothing here appears to be gcp only. merging.

@deads2k deads2k merged commit 83ac5ae into openshift:master Jun 29, 2018
@openshift-ci-robot
Copy link

@bparees: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/gcp 2a1b2d7 link /test gcp
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 3, 2018

This PR seems to have broken the logic during upgrades in openshift-ansible - oc adm policy reconcile-sccs --additive-only=true -o name now prints changes to the stdout during 3.10.n to 3.10.n+1 upgrades.

Is it expected or a side-effect of this PR?

@liggitt
Copy link
Contributor

liggitt commented Jul 3, 2018

I don't see anything in this PR that would have modified that. Are you passing --confirm?

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 3, 2018

I don't see anything in this PR that would have modified that

Weird, this error poped up approximately when this was merged.

Are you passing --confirm?

No, isn't it ignored now? openshift/openshift-ansible#8781 drops --confirm for storage.

I'll try adding --confirm flag

@liggitt
Copy link
Contributor

liggitt commented Jul 3, 2018

No, isn't it ignored now? openshift/openshift-ansible#8781 drops --confirm for storage.

that is a separate command. scc reconciliation still requires it.

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 3, 2018

Adding --confirm didn't help - openshift/openshift-ansible#9060.

In this test the upgrade is being run immediately after install, so its not the difference between minor versions. However I can't pinpoint the exact change which caused this, but the timing suggest its this PR

@sdodson
Copy link
Member

sdodson commented Jul 3, 2018

The ansible code that's failing is intended to abort the upgrade if there have been changes to bootstrapped SCCs because if we were to reconcile them we'd stomp on local customizations. So I believe this is an indication that something has changed unexpectedly?

@liggitt
Copy link
Contributor

liggitt commented Jul 3, 2018

Adding --confirm didn't help - openshift/openshift-ansible#9060.

I misunderstood the question. That code is intentionally doing a dry-run to see if reconciliation is required.

So I believe this is an indication that something has changed unexpectedly?

This PR added a new field to the SCCs that meant they needed reconciling. That is not unexpected. Why does that fail an upgrade?

@sdodson
Copy link
Member

sdodson commented Jul 3, 2018

openshift/openshift-ansible#8390 (and some subsequent cleanup) Added the check at the behest of support with feedback from @simo5
Current code https://github.com/openshift/openshift-ansible/blob/master/playbooks/openshift-master/private/upgrade.yml#L6-L27

@liggitt
Copy link
Contributor

liggitt commented Jul 3, 2018

openshift/openshift-ansible#8390 (and some subsequent cleanup) Added the check at the behest of support with feedback from @simo5
Current code https://github.com/openshift/openshift-ansible/blob/master/playbooks/openshift-master/private/upgrade.yml#L6-L27

that appears to be checking SCC reconciliation with an oc version skewed from the API server (new oc, old apiserver), which will emit false positives around reconciliation for this new defaulted field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/security size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants