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

Safe Settings doesn't run with just repos #572

Open
bheemvennapureddy opened this issue Dec 15, 2023 · 12 comments
Open

Safe Settings doesn't run with just repos #572

bheemvennapureddy opened this issue Dec 15, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@bheemvennapureddy
Copy link
Contributor

safesettings doesn't run with just using the repo/*.yml which means removing settings.yml and deployment-settings.yml
Screenshot 2023-12-15 at 2 27 27 PM
Screenshot 2023-12-15 at 2 27 46 PM

@bheemvennapureddy bheemvennapureddy added the bug Something isn't working label Dec 15, 2023
@bheemvennapureddy
Copy link
Contributor Author

@svg153 @decyjphr am i doing anything wrong ?

@bheemvennapureddy
Copy link
Contributor Author

Screenshot 2023-12-15 at 2 44 31 PM

@decyjphr
Copy link
Collaborator

It is coded to expect the settings.yml so it probably ends up as an error.

@bheemvennapureddy
Copy link
Contributor Author

bheemvennapureddy commented Dec 16, 2023

@decyjphr so we can't just rely on the repo's configuration and ignore the org config ?

@svg153
Copy link
Contributor

svg153 commented Dec 16, 2023

@decyjphr so we can't just rely on the repo's configuration and ignore the org config ?

It might be interesting to have a feature flag. Aren't you interested in having general configurations that apply to all repositories? or what is the reason? to know, curiosity

@bheemvennapureddy
Copy link
Contributor Author

each team has different reasons to have diff policies but i should be able to manage repo level policies

@bheemvennapureddy
Copy link
Contributor Author

What ever we have in the precedence order should be able to maintain them independently

@bheemvennapureddy
Copy link
Contributor Author

Looks like i also always need base branch protection rule without that i see this error

t;,"x-ratelimit-reset":"1702965036","x-ratelimit-resource":"core","x-ratelimit-used":"443","x-xss-protection":"0"},"data":{"message":"Invalid request.\\n\\nNo subschema in \\"anyOf\\" matched.\\nNo subschema in \\"oneOf\\" matched.\\nNot all subschemas of \\"allOf\\" matched.\\nFor 'anyOf/1', {\\"contexts\\"=>[\\"build\\", \\"codecov/patch\\", \\"validate_work_item\\", \\"actions\\"]} is not a null

@bheemvennapureddy
Copy link
Contributor Author

@svg153 @decyjphr any thoughts here ?

@svg153
Copy link
Contributor

svg153 commented Dec 19, 2023

Looks like i also always need base branch protection rule without that i see this error

t;,"x-ratelimit-reset":"1702965036","x-ratelimit-resource":"core","x-ratelimit-used":"443","x-xss-protection":"0"},"data":{"message":"Invalid request.\\n\\nNo subschema in \\"anyOf\\" matched.\\nNo subschema in \\"oneOf\\" matched.\\nNot all subschemas of \\"allOf\\" matched.\\nFor 'anyOf/1', {\\"contexts\\"=>[\\"build\\", \\"codecov/patch\\", \\"validate_work_item\\", \\"actions\\"]} is not a null

I do not have any protect rule in my test org and works correctly: https://github.com/svg153-org/admin/blob/main/.github/settings.yml

@decyjphr
Copy link
Collaborator

@svg153 @decyjphr any thoughts here ?

I tried to run a test without the settings.yml and I see that this line throws an error TypeError: Cannot read properties of undefined (reading 'data') at ConfigManager.loadYaml (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/lib/configManager.js:28:34) at async syncSettings (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/index.js:72:29) at async Promise.all (index 0) at async Promise.all (index 0) at async middleware (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/node_modules/@octokit/webhooks/dist-node/index.js:355:5) {stack: 'TypeError: Cannot read properties of undefine…s/@octokit/webhooks/dist-node/index.js:355:5)', message: 'Cannot read properties of undefined (reading 'data')'}

I am guessing that we'll need to fix this piece of code to ignore the file not being there and provide a empty object there.

Would you be able to modify this code? Or I can take a look at this later this week.

@bheemvennapureddy
Copy link
Contributor Author

@svg153 @decyjphr any thoughts here ?

I tried to run a test without the settings.yml and I see that this line throws an error TypeError: Cannot read properties of undefined (reading 'data') at ConfigManager.loadYaml (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/lib/configManager.js:28:34) at async syncSettings (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/index.js:72:29) at async Promise.all (index 0) at async Promise.all (index 0) at async middleware (/Users/decyjphr/projects/node/probot/safe-settings-oct-31/node_modules/@octokit/webhooks/dist-node/index.js:355:5) {stack: 'TypeError: Cannot read properties of undefine…s/@octokit/webhooks/dist-node/index.js:355:5)', message: 'Cannot read properties of undefined (reading 'data')'}

I am guessing that we'll need to fix this piece of code to ignore the file not being there and provide a empty object there.

Would you be able to modify this code? Or I can take a look at this later this week.

#576 is this what you mean ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants