Skip to content

Commit

Permalink
feat(require-quotes-in-ports-rule): handle "expose" section similar t…
Browse files Browse the repository at this point in the history
…o "ports"
  • Loading branch information
zavoloklom committed Nov 22, 2024
1 parent 6649594 commit 601a50f
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 91 deletions.
30 changes: 22 additions & 8 deletions docs/rules/require-quotes-in-ports-rule.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Require Quotes in Ports Rule

Ensures that the port values in the `ports` section of services in the Docker Compose file are enclosed in quotes. Using
quotes around port numbers can prevent YAML parsing issues and ensure that the ports are interpreted correctly.
Ensures that the port values in the `ports` and `expose` sections of services in the Docker Compose file are enclosed in
quotes. Using quotes around port numbers can prevent YAML parsing issues and ensure that the ports are interpreted
correctly.

This rule is fixable. The linter can automatically add the required quotes around port numbers without altering the
ports themselves. The type of quotes (single or double) can be configured via the `quoteType` option.
Expand All @@ -21,6 +22,8 @@ services:
ports:
- 80:80
- 443:443
expose:
- 3000
```
## Correct code example (Single Quotes)
Expand All @@ -32,6 +35,8 @@ services:
ports:
- '80:80'
- '443:443'
expose:
- '3000'
```
## Correct code example (Double Quotes)
Expand All @@ -43,17 +48,23 @@ services:
ports:
- "80:80"
- "443:443"
expose:
- "3000"
```
## Rule Details and Rationale
This rule ensures that the port numbers specified in the `ports` section of services are enclosed in quotes. Quoting
ports helps avoid potential issues with YAML parsing, where unquoted numbers might be misinterpreted or cause unexpected
behavior. By enforcing this rule, we ensure that the configuration is more robust and consistent.
This rule ensures that the port numbers specified in the `ports` and `expose` sections of services are enclosed in
quotes. Quoting ports helps avoid potential issues with YAML parsing, where unquoted numbers might be misinterpreted or
cause unexpected behavior. By enforcing this rule, we ensure that the configuration is more robust and consistent.

When mapping ports in the `HOST:CONTAINER` format, you may experience erroneous results when using a container port
lower than 60, because YAML parses numbers in the format `xx:yy` as a base-60 value. For this reason, we recommend
always explicitly specifying your port mappings as strings.
lower than 60, because YAML parses numbers in the format `xx:yy` as a [base-60 value](https://yaml.org/type/float.html).
For this reason, we recommend always explicitly specifying your port mappings as strings.

Although the expose section in Docker Compose does not suffer from the same YAML parsing vulnerabilities as the ports
section, it is still recommended to enclose exposed ports in quotes. Consistently using quotes across both `ports` and
`expose` sections creates a uniform configuration style, making the file easier to read and maintain.

## Options

Expand Down Expand Up @@ -85,8 +96,11 @@ If you want to enforce the use of double quotes around port mappings you can do

This rule was introduced in Docker-Compose-Linter [1.0.0](https://github.com/zavoloklom/docker-compose-linter/releases).

Handling `expose` section is added in [1.1.0](https://github.com/zavoloklom/docker-compose-linter/releases)

## References

- [Stackoverflow Discussion: Quotes on docker-compose.yml ports](https://stackoverflow.com/questions/58810789/quotes-on-docker-compose-yml-ports-make-any-difference)
- [Compose file reference](https://docker-docs.uclv.cu/compose/compose-file/#ports)
- [Compose File Reference: Ports](https://docker-docs.uclv.cu/compose/compose-file/#ports)
- [Compose File Reference: Expose](https://docs.docker.com/reference/compose-file/services/#expose)
- [Awesome Compose Examples](https://github.com/docker/awesome-compose)
83 changes: 50 additions & 33 deletions src/rules/require-quotes-in-ports-rule.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { parseDocument, isMap, isSeq, isScalar, Scalar, ParsedNode } from 'yaml';
import { parseDocument, isMap, isSeq, isScalar, Scalar, ParsedNode, Pair } from 'yaml';
import type {
LintContext,
LintMessage,
Expand All @@ -8,7 +8,7 @@ import type {
LintRuleSeverity,
RuleMeta,
} from '../linter/linter.types.js';
import { findLineNumberByValue } from '../util/line-finder.js';
import { findLineNumberForService } from '../util/line-finder.js';

interface RequireQuotesInPortsRuleOptions {
quoteType: 'single' | 'double';
Expand All @@ -24,29 +24,36 @@ export default class RequireQuotesInPortsRule implements LintRule {
public severity: LintRuleSeverity = 'minor';

public meta: RuleMeta = {
description: 'Ensure that ports are enclosed in quotes in Docker Compose files.',
description: 'Ensure that ports (in `ports` and `expose` sections) are enclosed in quotes in Docker Compose files.',
url: 'https://github.com/zavoloklom/docker-compose-linter/blob/main/docs/rules/require-quotes-in-ports-rule.md',
};

public fixable: boolean = true;

// eslint-disable-next-line class-methods-use-this
public getMessage(): string {
return 'Ports should be enclosed in quotes in Docker Compose files.';
return 'Ports in `ports` and `expose` sections should be enclosed in quotes.';
}

private readonly quoteType: 'single' | 'double';

private readonly portsSections: string[];

constructor(options?: RequireQuotesInPortsRuleOptions) {
this.quoteType = options?.quoteType || 'single';
this.portsSections = ['ports', 'expose'];
}

private getQuoteType(): Scalar.Type {
return this.quoteType === 'single' ? 'QUOTE_SINGLE' : 'QUOTE_DOUBLE';
}

// Static method to extract and process ports
private static extractPorts(document: ParsedNode | null, callback: (port: Scalar) => void) {
// Static method to extract and process values
private static extractValues(
document: ParsedNode | null,
section: string,
callback: (service: Pair, port: Scalar) => void,
) {
if (!document || !isMap(document)) return;

document.items.forEach((item) => {
Expand All @@ -56,14 +63,14 @@ export default class RequireQuotesInPortsRule implements LintRule {
serviceMap.items.forEach((service) => {
if (!isMap(service.value)) return;

const ports = service.value.items.find((node) => isScalar(node.key) && node.key.value === 'ports');
if (!ports || !isSeq(ports.value)) return;

ports.value.items.forEach((port) => {
if (isScalar(port)) {
callback(port);
}
});
const nodes = service.value.items.find((node) => isScalar(node.key) && node.key.value === section);
if (nodes && isSeq(nodes.value)) {
nodes.value.items.forEach((node) => {
if (isScalar(node)) {
callback(service, node);
}
});
}
});
});
}
Expand All @@ -72,20 +79,28 @@ export default class RequireQuotesInPortsRule implements LintRule {
const errors: LintMessage[] = [];
const parsedDocument = parseDocument(context.sourceCode);

RequireQuotesInPortsRule.extractPorts(parsedDocument.contents, (port) => {
if (port.type !== this.getQuoteType()) {
errors.push({
rule: this.name,
type: this.type,
category: this.category,
severity: this.severity,
message: this.getMessage(),
line: findLineNumberByValue(context.sourceCode, String(port.value)),
column: 1,
meta: this.meta,
fixable: this.fixable,
});
}
this.portsSections.forEach((section) => {
RequireQuotesInPortsRule.extractValues(parsedDocument.contents, section, (service, port) => {
if (port.type !== this.getQuoteType()) {
errors.push({
rule: this.name,
type: this.type,
category: this.category,
severity: this.severity,
message: this.getMessage(),
line: findLineNumberForService(
parsedDocument,
context.sourceCode,
String(service.key),
section,
String(port.value),
),
column: 1,
meta: this.meta,
fixable: this.fixable,
});
}
});
});

return errors;
Expand All @@ -94,11 +109,13 @@ export default class RequireQuotesInPortsRule implements LintRule {
public fix(content: string): string {
const parsedDocument = parseDocument(content);

RequireQuotesInPortsRule.extractPorts(parsedDocument.contents, (port) => {
if (port.type !== this.getQuoteType()) {
// eslint-disable-next-line no-param-reassign
port.type = this.getQuoteType();
}
this.portsSections.forEach((section) => {
RequireQuotesInPortsRule.extractValues(parsedDocument.contents, section, (service, port) => {
if (port.type !== this.getQuoteType()) {
// eslint-disable-next-line no-param-reassign
port.type = this.getQuoteType();
}
});
});

return parsedDocument.toString();
Expand Down
54 changes: 9 additions & 45 deletions src/util/line-finder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,42 +32,6 @@ function findLineNumberByValue(content: string, value: string): number {
return lineIndex === -1 ? 1 : lineIndex + 1;
}

/**
* Finds the line number where the key is located in the YAML content for a specific service.
* Searches only within the service block, ensuring boundaries are respected.
*
* @param document The YAML content parsed with lib yaml.
* @param content The parsed YAML content as a string.
* @param serviceName The name of the service in which to search for the key.
* @param key The key to search for in the content.
* @returns number The line number where the key is found, or -1 if not found.
*/
function findLineNumberByKeyForService(document: Document, content: string, serviceName: string, key: string): number {
const services = document.get('services') as Node;

if (!isMap(services)) {
return 1;
}

const service = services.get(serviceName) as Node;

if (!isMap(service)) {
return 1;
}

let lineNumber = 1;
service.items.forEach((item) => {
const keyNode = item.key;

if (isScalar(keyNode) && keyNode.value === key && keyNode.range) {
const [start] = keyNode.range;
lineNumber = content.slice(0, start).split('\n').length;
}
});

return lineNumber;
}

/**
* Refactored helper to get service block line number
*/
Expand Down Expand Up @@ -139,31 +103,31 @@ function findLineNumberForService(
}

if (isSeq(keyNode)) {
let line = 1;
keyNode.items.forEach((item) => {
if (isScalar(item) && item.value === value && item.range) {
if (isScalar(item) && String(item.value) === String(value) && item.range) {
const [start] = item.range;
return content.slice(0, start).split('\n').length;
line = content.slice(0, start).split('\n').length;
}

return 1;
});
return line;
}

if (isMap(keyNode)) {
let line = 1;
keyNode.items.forEach((item) => {
const keyItem = item.key;
const valueItem = item.value;

if (isScalar(keyItem) && isScalar(valueItem) && valueItem.value === value && valueItem.range) {
if (isScalar(keyItem) && isScalar(valueItem) && String(valueItem.value) === String(value) && valueItem.range) {
const [start] = valueItem.range;
return content.slice(0, start).split('\n').length;
line = content.slice(0, start).split('\n').length;
}

return 1;
});
return line;
}

return 1; // Default to 1 if the key or value is not found
}

export { findLineNumberByKey, findLineNumberByValue, findLineNumberByKeyForService, findLineNumberForService };
export { findLineNumberByKey, findLineNumberByValue, findLineNumberForService };
3 changes: 3 additions & 0 deletions tests/mocks/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ services:
ports:
- "11150:3000"
- "127.0.0.1:11032:3000"
- 3000
expose:
- 3000
b-service:
build:
context: ../../app/b-service
Expand Down
25 changes: 20 additions & 5 deletions tests/rules/require-quotes-in-ports-rule.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ test('RequireQuotesInPortsRule: should return a warning when ports are not quote

const errors = rule.check(context);
t.is(errors.length, 1, 'There should be one warning when ports are not quoted.');
t.is(errors[0].message, 'Ports should be enclosed in quotes in Docker Compose files.');
t.is(
errors[0].message,
'Ports in `ports` and `expose` sections should be enclosed in quotes.',
);
t.is(errors[0].rule, 'require-quotes-in-ports');
t.is(errors[0].severity, 'minor');
});
Expand Down Expand Up @@ -74,7 +77,10 @@ test('RequireQuotesInPortsRule: should fix unquoted ports by adding single quote
const rule = new RequireQuotesInPortsRule({ quoteType: 'single' });

const fixedYAML = rule.fix(yamlWithoutQuotes);
t.true(fixedYAML.includes(`'8080:80'`), 'The ports should be quoted with single quotes.');
t.true(
fixedYAML.includes(`'8080:80'`),
'The Ports in `ports` and `expose` sections should be quoted with single quotes.',
);
t.false(fixedYAML.includes('ports:\n - 8080:80'), 'The unquoted ports should no longer exist.');
});

Expand All @@ -83,7 +89,10 @@ test('RequireQuotesInPortsRule: should fix double quotes ports by changing them
const rule = new RequireQuotesInPortsRule({ quoteType: 'single' });

const fixedYAML = rule.fix(yamlWithSingleQuotes);
t.true(fixedYAML.includes(`'8080:80'`), 'The ports should be quoted with single quotes.');
t.true(
fixedYAML.includes(`'8080:80'`),
'The Ports in `ports` and `expose` sections should be quoted with single quotes.',
);
t.false(fixedYAML.includes(`"8080:80"`), 'The ports should not have double quotes.');
});

Expand All @@ -92,7 +101,10 @@ test('RequireQuotesInPortsRule: should fix unquoted ports by adding double quote
const rule = new RequireQuotesInPortsRule({ quoteType: 'double' });

const fixedYAML = rule.fix(yamlWithoutQuotes);
t.true(fixedYAML.includes(`"8080:80"`), 'The ports should be quoted with double quotes.');
t.true(
fixedYAML.includes(`"8080:80"`),
'The Ports in `ports` and `expose` sections should be quoted with double quotes.',
);
t.false(fixedYAML.includes('ports:\n - 8080:80'), 'The unquoted ports should no longer exist.');
});

Expand All @@ -101,6 +113,9 @@ test('RequireQuotesInPortsRule: should fix single quotes ports by changing them
const rule = new RequireQuotesInPortsRule({ quoteType: 'double' });

const fixedYAML = rule.fix(yamlWithSingleQuotes);
t.true(fixedYAML.includes(`"8080:80"`), 'The ports should be quoted with double quotes.');
t.true(
fixedYAML.includes(`"8080:80"`),
'The Ports in `ports` and `expose` sections should be quoted with double quotes.',
);
t.false(fixedYAML.includes(`'8080:80'`), 'The ports should not have single quotes.');
});

0 comments on commit 601a50f

Please sign in to comment.