open-policy-agent / conftest

Write tests against structured configuration data using the Open Policy Agent Rego query language
https://conftest.dev
Other
2.85k stars 301 forks source link

Bug: Accessing Rego Policies from Different Drives in Windows #979

Open pckvcode opened 1 month ago

pckvcode commented 1 month ago

bug: Accessing Rego Policies from Different Drives in Windows

Not able to access Rego policies from another drive in Windows. Error occurs while integrating with Golang.

Example: If go-binary/main.go is in the D drive and tries to access Rego policies located in the C drive (or vice-versa), an error occurs:

loading policies: load: 1 error occurred during loading: CreateFile \Users\pck\AppData\Local\Temp\temp_dir_12342789295736: The system cannot find the path specified.

OPA loader should identify Windows C, D drives and load files.

Quick Fix

Short Description

Steps To Reproduce

  1. Place Rego policy files in the C drive.
  2. Load policy from the D drive.
package main

import (
    "fmt"
    "os"

    opaPolicy "github.com/open-policy-agent/conftest/policy"
)

// Keep this code in D drive
// Create a directory in D drive and add dummy rego policies ex: D:\punith\opa-bug-reproduce
// Create a directory in C drive and add dummy rego policies ex: C:\Users\pck\AppData\Local\Temp\temp_dir_12342789295736
// go mod init opa-bug
// go get "github.com/open-policy-agent/conftest"
// go mod tidy
// go run main.go

func main() {
    currentWorkingDirectory, _ := os.Getwd()
    fmt.Println("Current Working Directory:", currentWorkingDirectory) // D:\punith\opa-bug-reproduce

    cDir := `C:\Users\pck\AppData\Local\Temp\temp_dir_12342789295736`
    dir := cDir
    _, err := opaPolicy.LoadWithData([]string{dir}, []string{dir}, "", false)
    if (err != nil) {
        fmt.Println("Error: error during loading with data", err)
        return
    }
    fmt.Println("Successfully loaded")
}

Expected Behavior

Code should be able to load Rego policies from other drives (C, D, or E drives) in Windows.

Additional Context

Bug in code: opa/loader.go#L575

Current:

if len(parts) == 2 && len(parts[0]) > 0 {

New:

if len(parts) == 2 && len(parts[0]) > 1 {

Description: It should not separate the prefix if it detects C/D/E drives.

New test case to be added: opa/loader_test.go#L825

{
  input:    `C:\foo\bar`,
  wantParts: []string{"foo", "bar"}
  wantPath: `C:\foo\bar`,
}

pckvcode commented 1 month ago

conftest project version used github.com/open-policy-agent/conftest v0.51.0

pckvcode commented 1 month ago

I encountered the same issue when using the conftest binary locally on Windows. Please find the attachment.

  1. conftest binary is in D drive
  2. list the file in D drive
  3. list the file in C drive
  4. verify the rego policy(d drive)
  5. verify the rego policy(c drive)
Screenshot 2024-08-05 at 5 30 31 PM
boranx commented 1 month ago

hi @pckvcode

Very nice find! The corresponding code block seems to be added from the very beginning of the OPA: https://github.com/open-policy-agent/opa/pull/176/files

With the current form, I can verify it ignores the C as the following:

example.com:C:/foo/bar

It should normally be able to open files using different drivers intuitively, so +1 from my side, though we need to check this strictly in unit tests-e2e to make sure it doesn't break any of the core features under the hood

Feel free to propose a PR containing multiple cases for unit-tests to OPA, and feel free to ping us in the PR to review

Thanks for your investigations

pckvcode commented 2 weeks ago

Earlier, there was an issue with loading policies using the path C:/path/to/policy/ on Windows. This was resolved by adding the file:/// prefix, so the policy path became file:///C:/path/to/policy/. For more details please refer this link Screenshot 2024-08-30 at 10 29 43 AM

However, we're now encountering an issue with loading the data.yaml file from other drives on Windows. I tried the following two options, but neither worked:

  1. file:///C:/path/to/data/data.yaml
  2. a:file:///C:/path/to/data/data.yaml

I've attached screenshots of the errors.

Screenshot 2024-08-30 at 10 31 39 AM

Command used:

.\conftest.exe test C:/Users/pck/Punith/test_files/config.yaml --policy file:///C:/Users/pck/Punith/test_files/policy --data C:/Users/pck/Punith/test_files/data.yaml

Steps to reproduce:

  1. Place the Rego policies and data.yaml in the C drive.
  2. Place the Conftest binary on the D drive.
  3. Run the following command:
    1. . \conftest.exe test C:/Users/pck/Punith/test_files/config.yaml --policy file:///C:/Users/pck/Punith/test_files/policy --data C:/Users/pck/Punith/test_files/data.yaml
    2. Error encountered: Error: running test: load: load documents: 1 error occurred during loading: CreateFile /Users/pck/Punith/test_files/data.yaml: The system cannot find the path specified.

It seems there may be an issue with how data files are loaded. Could we apply the same logic used for loading policy files here to loading data files here?

  1. Loading policy files: https://github.com/open-policy-agent/conftest/blob/a223c8396429e4b340dbd5fcfbcad1f105f6d89c/policy/engine.go#L72
  2. Loading data files: https://github.com/open-policy-agent/conftest/blob/a223c8396429e4b340dbd5fcfbcad1f105f6d89c/policy/engine.go#L137

More details:

Screenshot of files keeping in C drive

Screenshot 2024-08-30 at 10 32 34 AM
pckvcode commented 1 week ago

@boranx Any suggestion on the above comments