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

feat: List local and imported variables in the policy with InspectPolicies #2141

Merged
merged 9 commits into from
May 20, 2024

Conversation

oguzhand95
Copy link
Member

@oguzhand95 oguzhand95 commented May 7, 2024

InspectPolicies on disk store

{
    "results": {
        "derived_roles.common_roles": {
            "variables": [
                {
                    "name": "derivedRoleVariable",
                    "value": "request.resource.attr.isDerivedRoleVar",
                    "kind": "KIND_LOCAL",
                    "source": "derived_roles.common_roles"
                }
            ]
        },
        "export_variables.common_variables": {
            "variables": [
                {
                    "name": "commonMarkedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_EXPORTED",
                    "source": "export_variables.common_variables"
                }
            ]
        },
        "principal.john.vdefault": {
            "actions": [
                "*"
            ],
            "variables": [
                {
                    "name": "commonMarkedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_IMPORTED",
                    "source": "export_variables.common_variables"
                },
                {
                    "name": "markedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_LOCAL",
                    "source": "principal.john.vdefault"
                }
            ]
        },
        "resource.leave_request.vdefault": {
            "actions": [
                "*",
                "create",
                "duplicate",
                "view"
            ],
            "variables": [
                {
                    "name": "commonMarkedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_IMPORTED",
                    "source": "export_variables.common_variables"
                },
                {
                    "name": "markedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_LOCAL",
                    "source": "resource.leave_request.vdefault"
                }
            ]
        }
    }
}

cerbosctl inspect policies on disk store

POLICY ID                               ACTIONS                 VARIABLES                           
derived_roles.common_roles                                      derivedRoleVariable                     
export_variables.common_variables                               commonMarkedResource                    
principal.john.vdefault                 *                       commonMarkedResource,markedResource     
resource.leave_request.vdefault         *,create,duplicate,view commonMarkedResource,markedResource 

InspectPolicies on hub store

{
    "results": {
        "principal.john.vdefault": {
            "actions": [
                "*"
            ],
            "variables": [
                {
                    "name": "commonMarkedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_UNKNOWN"
                },
                {
                    "name": "markedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_UNKNOWN"
                }
            ]
        },
        "resource.leave_request.vdefault": {
            "actions": [
                "*",
                "create",
                "duplicate",
                "view"
            ],
            "variables": [
                {
                    "name": "commonMarkedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_UNKNOWN"
                },
                {
                    "name": "markedResource",
                    "value": "request.resource.attr.markedResource",
                    "kind": "KIND_UNKNOWN"
                }
            ]
        }
    }
}

cerbosctl inspect policies on hub store

POLICY ID                       ACTIONS                 VARIABLES                           
principal.john.vdefault         *                       commonMarkedResource,markedResource     
resource.leave_request.vdefault *,create,duplicate,view commonMarkedResource,markedResource

@oguzhand95 oguzhand95 self-assigned this May 7, 2024
@oguzhand95 oguzhand95 force-pushed the variables branch 4 times, most recently from fa2bced to 7a732f0 Compare May 8, 2024 11:06
@oguzhand95 oguzhand95 marked this pull request as ready for review May 8, 2024 11:27
@oguzhand95 oguzhand95 marked this pull request as draft May 9, 2024 10:18
api/public/cerbos/response/v1/response.proto Outdated Show resolved Hide resolved
api/public/cerbos/response/v1/response.proto Outdated Show resolved Hide resolved
api/public/cerbos/response/v1/response.proto Outdated Show resolved Hide resolved
docs/modules/api/pages/admin_api.adoc Outdated Show resolved Hide resolved
internal/policy/policy.go Outdated Show resolved Hide resolved
@oguzhand95 oguzhand95 force-pushed the variables branch 7 times, most recently from 2f17696 to 08de3ff Compare May 9, 2024 17:17
@oguzhand95 oguzhand95 marked this pull request as ready for review May 10, 2024 09:23
internal/policy/policy.go Show resolved Hide resolved
internal/policy/policy.go Outdated Show resolved Hide resolved
internal/policy/policy.go Outdated Show resolved Hide resolved
internal/policy/policy.go Outdated Show resolved Hide resolved
internal/policy/policy.go Outdated Show resolved Hide resolved
internal/storage/db/internal/db.go Outdated Show resolved Hide resolved
internal/inspect/internal/internal.go Outdated Show resolved Hide resolved
internal/inspect/internal/policy/inspect.go Outdated Show resolved Hide resolved
internal/inspect/internal/policy/inspect.go Outdated Show resolved Hide resolved
@oguzhand95 oguzhand95 force-pushed the variables branch 2 times, most recently from 5a5ef1b to 2cf5a72 Compare May 13, 2024 21:16
@oguzhand95
Copy link
Member Author

  • Refactored the code following your steps and it is simpler now.
  • Added used field to denote if a variable is used from a condition. However, the variables defined in the ExportVariables will not show up as used as you've stated. In the policy where they are used, they appear as imported and used.
  • If a variable is referenced from a condition, and the variable is nowhere to be found it is now marked as UNDEFINED.

I was querying the unfiltered and filtered policies from the database and still do. I use the unfiltered one to resolve the state of each variable so that we do not report any undefined variables in the inspection. If we only check the filtered policies while inspecting, then some variables may appear as undefined even if they are actually defined in the policies left out. I am not sure if this is the expected resolution though, let me know if this is not what you had in mind.

@charithe
Copy link
Contributor

I was querying the unfiltered and filtered policies from the database and still do. I use the unfiltered one to resolve the state of each variable

If there are a lot of policies, this would be quite inefficient. The list of filtered policies might not even be importing any variables -- in which case we would have done a lot of work for nothing.

Get the list of policies the user is interested in inspecting. While going through that list one by one, load and cache any imported variables as necessary. Or, do one pass, collect the set of imported policies that are referenced but not loaded initially, load all of them as a batch, and do a second pass to handle the unresolved variables.

@oguzhand95 oguzhand95 force-pushed the variables branch 2 times, most recently from 4ef169b to 2dd6513 Compare May 16, 2024 07:30
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/inspect/policyset.go Outdated Show resolved Hide resolved
internal/storage/hub/bundle.go Outdated Show resolved Hide resolved
…icies

Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
internal/inspect/policy.go Outdated Show resolved Hide resolved
internal/storage/index/index.go Outdated Show resolved Hide resolved
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
@oguzhand95 oguzhand95 enabled auto-merge (squash) May 20, 2024 09:23
@oguzhand95 oguzhand95 merged commit 2efb5e5 into cerbos:main May 20, 2024
22 checks passed
@oguzhand95 oguzhand95 deleted the variables branch May 20, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants