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

missing details in error log #202

Open
aarnaud opened this issue Sep 22, 2023 · 6 comments
Open

missing details in error log #202

aarnaud opened this issue Sep 22, 2023 · 6 comments
Labels
kind:enhancement New feature or request

Comments

@aarnaud
Copy link

aarnaud commented Sep 22, 2023

during a debug, I discovered that ksops error wasn't helpful, but sops error helped me to resolve my issue.
Is it possible to forward sops error to stderr ?

ksops

unable to generate manifests: error decrypting file "./secrets.yaml" from manifest.Files: trouble decrypting file: Error getting data key: 0 successful groups required,

sops:

Failed to get the data key required to decrypt the SOPS file.

Group 0: FAILED
  https://sops-xyz0987654321.vault.azure.net/keys/sops-key/1234567891234567891234567891234: FAILED
    - | failed to decrypt sops data key with Azure Key Vault key
      | 'https://sops-xyz0987654321.vault.azure.net/keys/sops-key/1234567891234567891234567891234':
      | POST
      | https://sops-xyz0987654321.vault.azure.net/keys/sops-key/1234567891234567891234567891234/decrypt
      | --------------------------------------------------------------------------------
      | RESPONSE 403: 403 Forbidden
      | ERROR CODE: Forbidden
      | --------------------------------------------------------------------------------
      | {
      |   "error": {
      |     "code": "Forbidden",
      |     "message": "Caller is not authorized to perform action
      | on resource.\r\nIf role assignments, deny assignments or
      | role definitions were changed recently, please observe
      | propagation time.\r\nCaller:
      | appid=xxxxxxxx-yyyy-zzzz-aaaa-bbbbbbbbbbb;oid=eeeeeeee-rrrrr-tttt-yyyy-uuuuuuuuuuu;iss=https://sts.windows.net/ffffffffffff-gggg-hhhh-jjjj-kkkkkkkkkk/\r\nAction:
      | 'Microsoft.KeyVault/vaults/keys/decrypt/action'\r\nResource:
      | '/subscriptions/067a55d3-b9c6-4cef-8d8d-a0ae46c1e1a6/resourcegroups/sops-key/providers/microsoft.keyvault/vaults/sops-xyz0987654321/keys/sops-key'\r\nAssignment:
      | (not found)\r\nDecisionReason: 'DeniedWithNoValidRBAC'
      | \r\nVault: sops-xyz0987654321;location=eastus\r\n",
      |     "innererror": {
      |       "code": "ForbiddenByRbac"
      |     }
      |   }
      | }
      | --------------------------------------------------------------------------------


Recovery failed because no master key was able to decrypt the file. In
order for SOPS to recover the file, at least one key has to be successful,
but none were.

@dosubot dosubot bot added the kind:enhancement New feature or request label Sep 22, 2023
@dosubot
Copy link

dosubot bot commented Sep 22, 2023

🤖

Thank you for bringing this to our attention. I agree that forwarding the SOPS error details to stderr would provide more helpful information for troubleshooting.

Currently, the KSOPS error handling for SOPS decryption failures is implemented in the decryptFile function in the KSOPS codebase. Here, if there's an error during decryption, it is caught and returned with a custom error message "trouble decrypting file". This error is then propagated up to the generate function where it is handled and an appropriate error message is printed.

func decryptFile(file string) ([]byte, error) {
	b, err := ioutil.ReadFile(file)
	if err != nil {
		return nil, fmt.Errorf("error reading %q: %w", file, err)
	}

	format := formats.FormatForPath(file)
	data, err := decrypt.DataWithFormat(b, format)
	if err != nil {
		return nil, fmt.Errorf("trouble decrypting file: %w", err)
	}
	return data, nil
}

To enhance the error logging, we could modify the error handling in the decryptFile function to include the original SOPS error details. This can be done by using the errors.Unwrap or errors.As functions to retrieve the original error details from the wrapped error.

I will create a pull request to implement this enhancement. Once it's reviewed and merged, this feature will be available in the next release of KSOPS. I'll update this issue when the pull request is ready for review.

Thank you for your valuable feedback and for helping us improve KSOPS!

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@aarnaud aarnaud changed the title missing detail in error log missing details in error log Sep 22, 2023
@devstein
Copy link
Collaborator

Hi @aarnaud, thanks for creating an issue! Dosu's suggestion looks great, although it can't create PRs (yet 😁).

If you're up for it, contributions are welcome! Otherwise, I'll make a PR in the next week or so

@aarnaud
Copy link
Author

aarnaud commented Sep 22, 2023

Hi @devstein

I just try, but it's maybe more complicate, because the details seems to be in https://github.com/getsops/sops/blob/main/usererrors.go#L23
It's not expose on ksops

@devstein
Copy link
Collaborator

devstein commented Sep 22, 2023

Thanks for investigating. We should be able to check if the error implements UserError which is public.

If it is, we can call UserError() to get the formatted error log

@devstein
Copy link
Collaborator

@dosu-bot Can you update the fix to decryptFile to check if the err implements the UserError interface from the getsops/sops/usererrors pacakge?

@dosubot
Copy link

dosubot bot commented Sep 28, 2023

🤖

Thank you for bringing this issue to our attention. It's great to see your interest in improving KSOPS!

To address the problem you mentioned, it does seem like updating the decryptFile function in the KSOPS codebase to check if the error implements the UserError interface from the getsops/sops/usererrors package would be a good solution. This would allow the detailed SOPS error messages to be forwarded to stderr in KSOPS, making it easier to troubleshoot issues.

I encourage you to go ahead and create a pull request with your proposed fix. The maintainers will review your changes and determine if they should be merged into the KSOPS codebase. Your contribution will be greatly appreciated by the KSOPS community!

If you have any further questions or need any assistance during the process of creating the pull request, feel free to ask. Happy contributing!


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants