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.

Sign up for GitHub

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

Jump to bottom

PANDARIA: Fix show unauthorized when group member to get the monitor data #10

Merged
merged 1 commit into from
Nov 19, 2020
Merged

PANDARIA: Fix show unauthorized when group member to get the monitor data #10

merged 1 commit into from
Nov 19, 2020

Conversation

GGGitBoy
Copy link
Collaborator

@GGGitBoy GGGitBoy commented Nov 12, 2020 •
edited
Loading

在通过方式 r.Header["Key"] 获取请求头数组时key需要遵循大驼峰的规范，否则返回的为nil

具体查看 golang/go#34799

golang 1.14 开始支持 r.Header.Values(key string) 方法获取请求头 map 对应的数组，但是当前 repo 还是golang 1.13。所以暂时修改 key X-RANCHER-GROUP 的命名

Relate issue：
https://github.com/cnrancher/pandaria/issues/1187

@aiwantaozi
Copy link

在代码旁边加一下comment和issue链接，其他没什么问题

orangedeng
orangedeng requested changes Nov 16, 2020
View reviewed changes
Copy link
Member

@orangedeng orangedeng left a comment

Choose a reason for hiding this comment

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

把user header key一起改了，不然只是为了规避问题只改group header有点奇怪

@GGGitBoy
Copy link
Collaborator Author

@orangedeng Update!

aiwantaozi
aiwantaozi reviewed Nov 18, 2020
View reviewed changes
Copy link

@aiwantaozi aiwantaozi left a comment

Choose a reason for hiding this comment

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

PR本身没什么问题，但是用户升级了rancher server之后，是不是监控就看不到数据了？是不是监控也需要升级，这个升级能否接受。

@GGGitBoy
Copy link
Collaborator Author

r.Header.Get是大小写不敏感的，有试过使用不同大小写的X-RANCHER-USER请求头，能获取到数据

@guangbochen
Copy link

@orangedeng @aiwantaozi can u please help to review it again, thanks

orangedeng
orangedeng approved these changes Nov 19, 2020
View reviewed changes
Copy link
Member

@orangedeng orangedeng left a comment

Choose a reason for hiding this comment

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

LGTM

@guangbochen guangbochen merged commit e2131c3 into cnrancher:master Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aiwantaozi aiwantaozi aiwantaozi approved these changes

@orangedeng orangedeng orangedeng approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GGGitBoy @aiwantaozi @guangbochen @orangedeng