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

fix: use supported apiVersion for deployment and authorization #150

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

sthaha
Copy link
Contributor

@sthaha sthaha commented Nov 29, 2021

From k8s version 1.22 the following apiVersions[1][2] are no longer served.
Hence the PR introduces the following changes

  • replace extensions/v1beta1 with apps/v1
  • replace authorization.k8s.io/v1beta1 with authorization.k8s.io/v1

[1]: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#tokenreview-v122
[2]: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#deployment-v116

Signed-off-by: Sunil Thaha sthaha@redhat.com

Fixes #151

From k8s version 1.22 the following apiVersions[1][2] are no longer served.
Hence the PR introduces the following changes
* replace `extensions/v1beta1` with `apps/v1`
* replace `authorization.k8s.io/v1beta1` with `authorization.k8s.io/v1`

[1]: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#tokenreview-v122
[2]: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#deployment-v116

Signed-off-by: Sunil Thaha <sthaha@redhat.com>
@sthaha sthaha force-pushed the fix-resource-attributes-example branch from b4f9509 to ef00e25 Compare November 29, 2021 04:34
@sthaha
Copy link
Contributor Author

sthaha commented Nov 29, 2021

cc: @paulfantom @jan--f @s-urbaniak , could you please take a look?

@ibihim
Copy link
Collaborator

ibihim commented Dec 1, 2021

Thank you for the effort! Why did you remove the non-resource example?

@s-urbaniak
Copy link
Collaborator

@sthaha what @ibihim suggested is correct. please don't remove the examples.

@sthaha
Copy link
Contributor Author

sthaha commented Dec 2, 2021

@s-urbaniak @ibihim I can submit a separate cleanup PR but since I have touched the examples, I thought I will clean up the duplicate examples/non-resource-url/non-resource-url as well which I think was added by mistake in 96d3359 .

In other words, after fixing the examples/non-resource-url/non-resource-url, and correcting the `kube-rbac-proxy version
to 0.11.0 in examples/non-resource/non-resource-url,

❯ diff examples/non-resource-url examples/non-resource-url/non-resource-url
Only in examples/non-resource-url: non-resource-url
diff examples/non-resource-url/README.md examples/non-resource-url/non-resource-url/README.md
5c5
< RBAC differentiates in two types, that need to be authorized, resources and non-resources. A resource request authorization, could for example be, that a requesting entity needs to be authorized to perform the `get` action on a particular Kubernetes Deployment. A non-resource authorization validates, that an entity is authorized to request a bare URL.
---
> RBAC differentiates in two types, that need to be authorized, resources and non-resoruces. A resource request authorization, could for example be, that a requesting entity needs to be authorized to perform the `get` action on a particular Kubernetes Deployment. A non-resource authorization validates, that an entity is authorized to request a bare URL.

which is a typo in non-resoruces

@ibihim
Copy link
Collaborator

ibihim commented Dec 2, 2021

@sthaha, I think it is amazing, that you continue to support the repository beyond the version bump, but could you add the 3 files back for this PR?

+73 -325 << 3 files get deleted: examples/non-resource-url/non-resource-url/*

@jan--f
Copy link
Contributor

jan--f commented Dec 2, 2021

As @sthaha already pointed out, the removed files seem to have been added erroneously and only differ in that they have a few typos in them compared the actual example files. So 👍 for removing those.
This LGTM!

@ibihim
Copy link
Collaborator

ibihim commented Dec 2, 2021

Ah, sorry. My bad. I completely misunderstood the reasoning. /lgtm

I actually was wondering about it the day before (folder in folder). 😅

@s-urbaniak
Copy link
Collaborator

ah i see, now i also remember @jan--f talking about this :-) lgtm, let's ship

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.

Examples no longer work on k8s 1.22.0
4 participants