-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
We should add tests cases for grant/revoke and permit/deny with absolute and relative paths in the test fixtures: https://github.com/cyberark/conjur-policy-parser/tree/master/spec/resolver-fixtures There should at least be test case that would fail without this before making the change. |
Yes, I will add a test case. This PR was put on hold until I update master to match the current tag (v3.0.4) |
3812964
to
9385661
Compare
9385661
to
ca4b82b
Compare
- !revoke | ||
member: !member | ||
account: the-account | ||
id: myapp/../the-group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you expect this ..
to be expanded/resolved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not, I forgot to change this from an earlier test I was running.
lib/conjur/policy/resolver.rb
Outdated
Array(record.member).each do |member| | ||
member.role.id = absolute_path_of(member.role.id) | ||
# Member has either role.id or id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment should be expanded a bit more. It's not clear to me what each of these cases means.
ca4b82b
to
adb3738
Compare
This should fail when running `./test.sh` because the fix has not been implemented yet.
adb3738
to
f832ca2
Compare
Issue: #22
I was unsure regarding the relative paths for members in
revoke
andgrant
statements. It seems that members either have arole.id
or anid
. So currently, I transform both of those into relative paths. (Not sure if that was the right decision)The next step is to bump the version!