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

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

Open
pckvcode opened this issue Aug 5, 2024 · 5 comments · May be fixed by #999 or #1008
Open

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

pckvcode opened this issue Aug 5, 2024 · 5 comments · May be fixed by #999 or #1008
Labels
bug Something isn't working

Comments

@pckvcode
Copy link

pckvcode commented Aug 5, 2024


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

  • OPA version: v0.67.0
  • Environment: Windows 11
  • Error:
    loading policies: load: 1 error occurred during loading: CreateFile \Users\pck\AppData\Local\Temp\temp_dir_12342789295736: The system cannot find the path specified.
    
  • System type: x64 based PC

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
Copy link
Author

pckvcode commented Aug 5, 2024

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

@pckvcode
Copy link
Author

pckvcode commented Aug 5, 2024

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 boranx added the bug Something isn't working label Aug 6, 2024
@boranx
Copy link
Member

boranx commented Aug 8, 2024

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 added a commit to pckvcode/opa-pckvcode that referenced this issue Aug 12, 2024
…ther drives in Windows. For more details, see open-policy-agent#6910 and open-policy-agent/conftest#979.

OPA should be capable of accessing Rego policies from specified paths including drives (e.g., c:\a\b\c.rego).

The code has been updated to allow OPA to load Rego policies from paths with drives. Now, OPA can load Rego policies from paths, URLs, and drives.

Fixes open-policy-agent#6910

Signed-off-by: pckvcode <[email protected]>
@pckvcode
Copy link
Author

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:
    policies, err := loader.NewFileLoader().WithProcessAnnotation(true).Filtered(policyPaths, func(_ string, info os.FileInfo, _ int) bool {
  2. Loading data files:
    documents, err := loader.NewFileLoader().All(allDocumentPaths)

More details:

Screenshot of files keeping in C drive
Screenshot 2024-08-30 at 10 32 34 AM

@pckvcode
Copy link
Author

pckvcode commented Sep 3, 2024

@boranx
Any suggestion on the above comments

pckvcode pushed a commit to pckvcode/conftest-pck that referenced this issue Sep 3, 2024
    Conftest encounters errors on Windows when loading file paths that include drive letters (e.g., `C:/path/to/data.yaml`).
    Even when using a file URL (e.g., `file:///C:/path/to/data.yaml`), we still face issues.
    With these code changes, Conftest can now successfully load files using a file URL (e.g., `file:///C:/path/to/data.yaml`).
    We opted for file URLs instead of paths with drive letters (e.g., `C:/path/to/data.yaml`) because OPA does not support file paths with drive letters. For more details, see [this issue comment](open-policy-agent/opa#6922 (comment)).

    Resolves: open-policy-agent#979

Signed-off-by: Punith C K <[email protected]>
pckvcode pushed a commit to pckvcode/conftest-pck that referenced this issue Sep 23, 2024
… file:///C:/path/to/data.yaml) on windows

Conftest encounters errors on Windows when loading file paths that include drive letters (e.g., C:/path/to/data.yaml). Even when using a file URL (e.g., file:///C:/path/to/data.yaml), we still face issues.
We opted for file URLs(e.g., file:///C:/path/to/data.yaml) instead of paths with drive letters (e.g., C:/path/to/data.yaml) because OPA does not support file paths with drive letters.
Resolves open-policy-agent#979
pckvcode pushed a commit to pckvcode/conftest-pck that referenced this issue Sep 23, 2024
… file:///C:/path/to/data.yaml) on windows

Conftest encounters errors on Windows when loading file paths that include drive letters (e.g., C:/path/to/data.yaml). Even when using a file URL (e.g., file:///C:/path/to/data.yaml), we still face issues.
We opted for file URLs(e.g., file:///C:/path/to/data.yaml) instead of paths with drive letters (e.g., C:/path/to/data.yaml) because OPA does not support file paths with drive letters.
Resolves open-policy-agent#979

Signed-off-by: Punith C K <[email protected]>
pckvcode pushed a commit to pckvcode/conftest-pck that referenced this issue Oct 17, 2024
… file:///C:/path/to/data.yaml) on windows

    Conftest encounters errors on Windows when loading file paths that include drive letters (e.g., C:/path/to/data.yaml). Even when using a file URL (e.g., file:///C:/path/to/data.yaml), we still face issues.
    We opted for file URLs(e.g., file:///C:/path/to/data.yaml) instead of paths with drive letters (e.g., C:/path/to/data.yaml) because OPA does not support file paths with drive letters.
    Resolves open-policy-agent#979

Signed-off-by: Punith C K <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment