-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore(deps): do not attempt to update nginx #2017
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
{ | ||
"extends": [ | ||
"config:best-practices", | ||
":automergeStableNonMajor" | ||
], | ||
"packageRules": [ | ||
{ | ||
"allowedVersions": "1.9.8", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since 'allowedVersions' only allows a single version, automatically updating to new versions of the package will not be possible. This could potentially cause compatibility and security issues further down the road because it prevents the use of new updates that may include fixes for bugs or security vulnerabilities. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoding a specific version (1.9.8) could lead to usage of outdated versions over time. Instead of hardcoding, a range of allowed versions or a policy to upgrade at intervals whilst maintaining compatibility could be a better approach. |
||
"matchDatasources": [ | ||
"docker" | ||
], | ||
"matchPackageNames": [ | ||
// Prevent automatic updates from nginx:1.9.8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commenting within a JSON file is not standard and may cause issues in some parsers. You should consider removing the comments or move any important clarifications to your project's documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having user/customer-specific info (like Windows XP support) in the codebase can be misleading and introduce the potential for unwanted dependencies and confusion. Consider moving this architectural information to your documentation or a different, more explanatory format. |
||
// While this image can still be found on DockerHub, | ||
// it is also archived at https://archive.org/details/nginx_1.9.8.tar | ||
// This is required to serve as a proxy to the [SChannel](https://web.archive.org/web/20230402130420/https://learn.microsoft.com/en-us/windows/win32/com/schannel) that Windows XP uses. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URL in comments should be configured or parameterized in code or formal configuration files. This can make updating easier and also keep track of changes via version control system. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments inside a JSON file is not a JSON standard and might not be supported by all parsers. This might create compatibility issues. It's recommended to move these comments to the appropriate technical documentation/resources or use an extension with built-in support for comments, like JSONC. |
||
// Windows XP is required support since the legacy client can not be upgraded or modified. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Supporting Windows XP which is a deprecated operating system is generally not recommended as it may require old and possibly unsupported software. Upgrading systems would be a better solution as it eliminates vulnerabilities and other issues associated with legacy software. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the mentioned support of Windows XP OS and legacy client is very important, your codebase should include automated tests to ensure continual compatibility. This is important to note as current configuration file does not reflect any mechanism for testing Windows XP support. |
||
"nginx" | ||
] | ||
} | ||
], | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a widely accepted practice to end files with a newline character. Some tools might not process the file correctly without this. Although it's not critical, consider including a final newline to adhere to best practices. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a good practice to end a file with a newline character, to avoid unexpected issues with different tools and environments. Most editors can automatically add the newline on save. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The JSON object lacks a comma at the end of the rules array. This could cause parsing issues. It might be a simple typo, but it's good to keep an eye on consistent formatting to prevent parsing errors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good practice to end a file with a newline to maintain compatibility across various text editors and command line tools. Would suggest adding a newline at the end of the file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locking the allowed version to a very specific and old version (1.9.8) is risky - security issues found in this version will not receive fixes. This may expose your system to potential vulnerabilities. Therefore, it is a best practice to ensure you are using an updated version provided the legacy system can handle it.