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

Extend SetHeader Requests method ito accept multiple values #50639

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Extend SetHeader Requests method ito accept multiple values #50639

merged 1 commit into from
Aug 15, 2017

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Aug 14, 2017

This allows to set headers that are multivalued directly.
The headers variable is not directly accessible and currently
SetHeaders allows to set only one value.

NONE

This allows to set headers that are multivalued directly.
The headers variable is not directly accessible and currently
SetHeaders allows to set only one value.

Signed-off-by: Simo Sorce <[email protected]>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 14, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @simo5. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 14, 2017
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 14, 2017
@simo5
Copy link
Contributor Author

simo5 commented Aug 14, 2017

@enj PTAL

@grodrigues3
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 14, 2017
@enj
Copy link
Member

enj commented Aug 15, 2017

/lgtm
/assign @deads2k

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2017
@deads2k
Copy link
Contributor

deads2k commented Aug 15, 2017

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, enj, simo5

Associated issue requirement bypassed by: deads2k

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 15, 2017
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@enj
Copy link
Member

enj commented Aug 15, 2017

/retest

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 50023, 50639)

@k8s-github-robot k8s-github-robot merged commit 039f898 into kubernetes:master Aug 15, 2017
openshift-merge-robot added a commit to openshift/origin that referenced this pull request Aug 17, 2017
Automatic merge from submit-queue

Migrate to Kubernetes RBAC

Trello xref: https://trello.com/c/n3bR3Ys9

Fixes #12303
Fixes #13549
Fixes #13432
Fixes #15338
Fixes #14168
Fixes #10056

Need to investigate:

- [x] ...

Dependencies:

- [x] Prerequisite #15342
- [x] Requires openshift/openshift-ansible/pull/4933 @sdodson 
- [x] Blocked on openshift/openshift-ansible/issues/4967
- [x] Prerequisite kubernetes/kubernetes#50639

Followups:
- [ ] #15412
- [ ] #13316
- [ ] #13156
- [ ] #13430
- [ ] Should delete with proxy return details?
- [ ] Make project creation use RBAC instead of proxy endpoints?
- [ ] Remove policy objects from bootstrap roles
- [ ] Check if delegated_test.go can be revived
- [ ] Check to see if the deleted unit tests are reflected upstream and fix gaps
- [ ] Open issue to remove `openshiftSubjectLocator`
- [ ] Open issue to revisit forbidden message maker
- [ ] Update upstream `subject_locator_test` with origin's extensive testing
- [ ] Fix proxied create: ` _ bool is includeUnintialized, which we should really be passing through to the underlying API... it's odd there's not a CreateOptions parameter to Create`
- [ ] Fix proxied update: `if initializers use Update() to initialize objects (which I think they do), we may need to pass GetOptions{IncludeUninitialized: true} here...`
- [ ] Fix panics() in Convert...OrDie() functions
- [ ] glog.Fatal on post stark hook error
- [ ] Remove `TestPolicyCache`?
- [ ] Use discovery API based gating?
- [ ] upstream rules have always required a group. followup issue to remove getAPIGroupLegacy from `pkg/authorization/authorizer/scope/converter.go`
- [ ] issue to remove "normalizeResources" from `pkg/cmd/server/bootstrappolicy/policy.go`
- [ ] issue to  find callers of `clusterpolicyregistry "github.com/openshift/origin/pkg/authorization/registry/clusterpolicy"` and move to point of use
- [ ] issue to switch our encoding to rbac in `pkg/cmd/server/admin/create_bootstrappolicy_file.go`
- [ ] Exercise proxied endpoints
- [ ] hack/test-cmd.sh of gated overwrite bootstrap policy
- [ ] Delete unused legacy policy registry code
- [ ] Make RBAC discovery rule authoritative `pkg/authorization/apis/authorization/types.go`
- [ ] Fix `ignoreError` in `pkg/oc/admin/router/router.go`
- [ ] Confirm changes to `TestAuthorizationResolution` and `TestAuthorizationResourceAccessReview` in `test/integration/authorization_test.go`

Done:

- Store ClusterRoles as native RBAC Objects via Kubernetes.
- Provides backwards compatible API for the old policy based roles.
- Use Kubernetes authorizer

TODO:

- [x] Delete policy end points
- [x] Decide what to do with overwrite policy
- [x] Remove or gate `oc create policybinding`
- [x] Move new impersonation code to `pkg/auth/client/impersonate.go`
- [x] Remove any unnecessary conversions
- [x] Review new `proxy.go` files
- [x] Remove reason logic `allowed by rule in ...`
- [x] Add interface assertion to proxy files
- [x] Confirm we need `pkg/authorization/util/convert/convert.go`
- [x] Confrim we need to expose some of the private conversion functions
- [x] Add protect/autoupdate annotation conversion to general conversion functions
- [x] ~~Support watch on proxied endpoints~~
- [x] Cherry pick kubernetes/kubernetes#49868 -> #15721
- [x] Fix upstream commits
- [x] Restore and version gate `NewCmdMigrateAuthorization`
- [x] ~~Wrap other errors in proxy files?~~ Remove all error wrapping
- [x] Make `NewImpersonatingRBACFromContext` more generic
- [x] Kube authorizer's reason on deny contains evaluation errors - do we want to preserve those?
- [x] Review `ImpersonatingRESTClient` in `pkg/auth/client/impersonate.go`
- [ ] Review `pkg/project/auth/cache.go` and ` pkg/project/auth/cache_test.go`
- [ ] Review ` pkg/authorization/authorizer/scope/converter_test.go`
- [ ] Review `k8s.io/kubernetes/staging/src/k8s.io/client-go/rest/request.go`
@simo5
Copy link
Contributor Author

simo5 commented Aug 21, 2017

@deads2k PTAL

@simo5
Copy link
Contributor Author

simo5 commented Aug 21, 2017

@deads2k sorry my browser did not refresh ignore

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants