Skip to content

Commit

Permalink
fix version sorting and report major version in outdated (#670)
Browse files Browse the repository at this point in the history
* fix version sorting and report major version in outdated

* convert wantedMajor and latestMajor to string

* update test to expect just the full semver version in the publishedVersions object

* break into a new function

* update test and some names for clarity

* more consistent naming
  • Loading branch information
joshspicer authored Nov 8, 2023
1 parent 5a5a9b2 commit 88cbc17
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 76 deletions.
39 changes: 25 additions & 14 deletions src/spec-configuration/containerCollectionsOCI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,28 @@ async function getJsonWithMimeType<T>(params: CommonParams, url: string, ref: OC
}
}

// Lists published versions/tags of a feature/template
// Gets published tags and sorts them by ascending semantic version.
// Omits any tags (eg: 'latest', or major/minor tags '1','1.0') that are not semantic versions.
export async function getVersionsStrictSorted(params: CommonParams, ref: OCIRef): Promise<string[] | undefined> {
const { output } = params;

const publishedTags = await getPublishedTags(params, ref);
if (!publishedTags) {
return;
}

const sortedVersions = publishedTags
.filter(f => semver.valid(f)) // Remove all major,minor,latest tags
.sort((a, b) => semver.compare(a, b));

output.write(`Published versions (sorted) for '${ref.id}': ${JSON.stringify(sortedVersions, undefined, 2)}`, LogLevel.Trace);

return sortedVersions;
}

// Lists published tags of a Feature/Template
// Specification: https://github.com/opencontainers/distribution-spec/blob/v1.0.1/spec.md#content-discovery
export async function getPublishedVersions(params: CommonParams, ref: OCIRef, sorted: boolean = false): Promise<string[] | undefined> {
export async function getPublishedTags(params: CommonParams, ref: OCIRef): Promise<string[] | undefined> {
const { output } = params;
try {
const url = `https://${ref.registry}/v2/${ref.namespace}/${ref.id}/tags/list`;
Expand Down Expand Up @@ -470,18 +489,10 @@ export async function getPublishedVersions(params: CommonParams, ref: OCIRef, so

const publishedVersionsResponse: OCITagList = JSON.parse(body);

if (!sorted) {
return publishedVersionsResponse.tags;
}

// Sort tags in descending order, removing latest.
const hasLatest = publishedVersionsResponse.tags.includes('latest');
const sortedVersions = publishedVersionsResponse.tags
.filter(f => f !== 'latest')
.sort((a, b) => semver.compareIdentifiers(a, b));


return hasLatest ? ['latest', ...sortedVersions] : sortedVersions;
// Return published tags from the registry as-is, meaning:
// - Not necessarily sorted
// - *Including* major/minor/latest tags
return publishedVersionsResponse.tags;
} catch (e) {
output.write(`Failed to parse published versions: ${e}`, LogLevel.Error);
return;
Expand Down
6 changes: 4 additions & 2 deletions src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Log, LogLevel } from '../spec-utils/log';
import { request } from '../spec-utils/httpRequest';
import { fetchOCIFeature, tryGetOCIFeatureSet, fetchOCIFeatureManifestIfExistsFromUserIdentifier } from './containerFeaturesOCI';
import { uriToFsPath } from './configurationCommonUtils';
import { CommonParams, ManifestContainer, OCIManifest, OCIRef, getPublishedVersions, getRef } from './containerCollectionsOCI';
import { CommonParams, ManifestContainer, OCIManifest, OCIRef, getRef, getVersionsStrictSorted } from './containerCollectionsOCI';
import { Lockfile, readLockfile, writeLockfile } from './lockfile';
import { computeDependsOnInstallationOrder } from './containerFeaturesOrder';
import { logFeatureAdvisories } from './featureAdvisories';
Expand Down Expand Up @@ -591,7 +591,7 @@ export async function loadVersionInfo(params: ContainerFeatureInternalParams, co
const updatedFeatureId = getBackwardCompatibleFeatureId(output, userFeatureId);
const featureRef = getRef(output, updatedFeatureId);
if (featureRef) {
const versions = (await getPublishedVersions(params, featureRef, true))
const versions = (await getVersionsStrictSorted(params, featureRef))
?.reverse();
if (versions) {
const lockfileVersion = lockfile?.features[userFeatureId]?.version;
Expand All @@ -613,7 +613,9 @@ export async function loadVersionInfo(params: ContainerFeatureInternalParams, co
features[userFeatureId] = {
current: lockfileVersion || wanted,
wanted,
wantedMajor: wanted && semver.major(wanted)?.toString(),
latest: versions[0],
latestMajor: semver.major(versions[0])?.toString(),
};
}
}
Expand Down
30 changes: 15 additions & 15 deletions src/spec-node/collectionCommonUtils/publishCommandImpl.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import path from 'path';
import * as semver from 'semver';
import { Log, LogLevel } from '../../spec-utils/log';
import { CommonParams, getPublishedVersions, OCICollectionRef, OCIRef } from '../../spec-configuration/containerCollectionsOCI';
import { CommonParams, getPublishedTags, OCICollectionRef, OCIRef } from '../../spec-configuration/containerCollectionsOCI';
import { OCICollectionFileName } from './packageCommandImpl';
import { pushCollectionMetadata, pushOCIFeatureOrTemplate } from '../../spec-configuration/containerCollectionsOCIPush';

let semanticVersions: string[] = [];
function updateSemanticVersionsList(publishedVersions: string[], version: string, range: string, publishVersion: string) {
function updateSemanticTagsList(publishedTags: string[], version: string, range: string, publishVersion: string) {
// Reference: https://github.com/npm/node-semver#ranges-1
const publishedMaxVersion = semver.maxSatisfying(publishedVersions, range);
const publishedMaxVersion = semver.maxSatisfying(publishedTags, range);
if (publishedMaxVersion === null || semver.compare(version, publishedMaxVersion) === 1) {
semanticVersions.push(publishVersion);
}
return;
}

export function getSemanticVersions(version: string, publishedVersions: string[], output: Log) {
if (publishedVersions.includes(version)) {
export function getSemanticTags(version: string, tags: string[], output: Log) {
if (tags.includes(version)) {
output.write(`(!) WARNING: Version ${version} already exists, skipping ${version}...`, LogLevel.Warning);
return undefined;
}
Expand All @@ -31,10 +31,10 @@ export function getSemanticVersions(version: string, publishedVersions: string[]

// Adds semantic versions depending upon the existings (published) versions
// eg. 1.2.3 --> [1, 1.2, 1.2.3, latest]
updateSemanticVersionsList(publishedVersions, version, `${parsedVersion.major}.x.x`, `${parsedVersion.major}`);
updateSemanticVersionsList(publishedVersions, version, `${parsedVersion.major}.${parsedVersion.minor}.x`, `${parsedVersion.major}.${parsedVersion.minor}`);
updateSemanticTagsList(tags, version, `${parsedVersion.major}.x.x`, `${parsedVersion.major}`);
updateSemanticTagsList(tags, version, `${parsedVersion.major}.${parsedVersion.minor}.x`, `${parsedVersion.major}.${parsedVersion.minor}`);
semanticVersions.push(version);
updateSemanticVersionsList(publishedVersions, version, `x.x.x`, 'latest');
updateSemanticTagsList(tags, version, `x.x.x`, 'latest');

return semanticVersions;
}
Expand All @@ -43,24 +43,24 @@ export async function doPublishCommand(params: CommonParams, version: string, oc
const { output } = params;

output.write(`Fetching published versions...`, LogLevel.Info);
const publishedVersions = await getPublishedVersions(params, ociRef);
const publishedTags = await getPublishedTags(params, ociRef);

if (!publishedVersions) {
if (!publishedTags) {
return;
}

const semanticVersions: string[] | undefined = getSemanticVersions(version, publishedVersions, output);
const semanticTags: string[] | undefined = getSemanticTags(version, publishedTags, output);

if (!!semanticVersions) {
output.write(`Publishing versions: ${semanticVersions.toString()}...`, LogLevel.Info);
if (!!semanticTags) {
output.write(`Publishing tags: ${semanticTags.toString()}...`, LogLevel.Info);
const pathToTgz = path.join(outputDir, archiveName);
const digest = await pushOCIFeatureOrTemplate(params, ociRef, pathToTgz, semanticVersions, collectionType, featureAnnotations);
const digest = await pushOCIFeatureOrTemplate(params, ociRef, pathToTgz, semanticTags, collectionType, featureAnnotations);
if (!digest) {
output.write(`(!) ERR: Failed to publish ${collectionType}: '${ociRef.resource}'`, LogLevel.Error);
return;
}
output.write(`Published ${collectionType}: '${ociRef.id}'`, LogLevel.Info);
return { publishedVersions: semanticVersions, digest };
return { publishedTags: semanticTags, digest };
}

return {}; // Not an error if no versions were published, likely they just already existed and were skipped.
Expand Down
18 changes: 9 additions & 9 deletions src/spec-node/featuresCLI/info.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Argv } from 'yargs';
import { OCIManifest, OCIRef, fetchOCIManifestIfExists, getPublishedVersions, getRef } from '../../spec-configuration/containerCollectionsOCI';
import { OCIManifest, OCIRef, fetchOCIManifestIfExists, getPublishedTags, getRef } from '../../spec-configuration/containerCollectionsOCI';
import { Log, LogLevel, mapLogLevel } from '../../spec-utils/log';
import { getPackageConfig } from '../../spec-utils/product';
import { createLog } from '../devContainers';
Expand Down Expand Up @@ -27,7 +27,7 @@ export function featuresInfoHandler(args: FeaturesInfoArgs) {
interface InfoJsonOutput {
manifest?: OCIManifest;
canonicalId?: string;
publishedVersions?: string[];
publishedTags?: string[];
}

async function featuresInfo({
Expand Down Expand Up @@ -86,12 +86,12 @@ async function featuresInfo({

// --- Get all published tags for resource
if (mode === 'tags' || mode === 'verbose') {
const publishedVersions = await getTags(params, featureRef);
const publishedTags = await getTags(params, featureRef);
if (outputFormat === 'text') {
console.log(encloseStringInBox('Published Version'));
console.log(`${publishedVersions.join('\n ')}`);
console.log(encloseStringInBox('Published Tags'));
console.log(`${publishedTags.join('\n ')}`);
} else {
jsonOutput.publishedVersions = publishedVersions;
jsonOutput.publishedTags = publishedTags;
}
}

Expand Down Expand Up @@ -145,16 +145,16 @@ async function getManifest(params: { output: Log; env: NodeJS.ProcessEnv; output

async function getTags(params: { output: Log; env: NodeJS.ProcessEnv; outputFormat: string }, featureRef: OCIRef) {
const { outputFormat } = params;
const publishedVersions = await getPublishedVersions(params, featureRef, true);
if (!publishedVersions || publishedVersions.length === 0) {
const publishedTags = await getPublishedTags(params, featureRef);
if (!publishedTags || publishedTags.length === 0) {
if (outputFormat === 'json') {
console.log(JSON.stringify({}));
} else {
console.log(`No published versions found for feature '${featureRef.resource}'\n`);
}
process.exit(1);
}
return publishedVersions;
return publishedTags;
}

function encloseStringInBox(str: string, indent: number = 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/spec-node/featuresCLI/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async function featuresPublish({
process.exit(1);
}

const isPublished = (publishResult?.digest && publishResult?.publishedVersions.length > 0);
const isPublished = (publishResult?.digest && publishResult?.publishedTags.length > 0);
let thisResult = isPublished ? {
...publishResult,
version: f.version,
Expand All @@ -126,7 +126,7 @@ async function featuresPublish({
process.exit(1);
}

if (publishResult?.digest && publishResult?.publishedVersions.length > 0) {
if (publishResult?.digest && publishResult?.publishedTags.length > 0) {
publishedLegacyIds.push(legacyId);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/spec-node/templatesCLI/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ async function templatesPublish({
process.exit(1);
}

const thisResult = (publishResult?.digest && publishResult?.publishedVersions?.length > 0) ? {
const thisResult = (publishResult?.digest && publishResult?.publishedTags?.length > 0) ? {
...publishResult,
version: t.version,
} : {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"ghcr.io/devcontainers/features/git:1.0": "latest",
"ghcr.io/devcontainers/features/git-lfs@sha256:24d5802c837b2519b666a8403a9514c7296d769c9607048e9f1e040e7d7e331c": "latest",
"ghcr.io/devcontainers/features/github-cli": "latest",
"ghcr.io/devcontainers/features/azure-cli:0": "latest"
"ghcr.io/devcontainers/features/azure-cli:0": "latest",
"ghcr.io/codspace/versioning/foo:0.3.1": "latest"
}
}
24 changes: 12 additions & 12 deletions src/test/container-features/containerFeaturesOCIPush.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const output = makeLog(createPlainLog(text => process.stdout.write(text),
const testAssetsDir = `${__dirname}/assets`;

interface PublishResult {
publishedVersions: string[];
publishedTags: string[];
digest: string;
version: string;
publishedLegacyIds?: string[];
Expand Down Expand Up @@ -87,7 +87,7 @@ registry`;
const color = result['color'];
assert.isDefined(color);
assert.isDefined(color.digest);
assert.deepEqual(color.publishedVersions, [
assert.deepEqual(color.publishedTags, [
'1',
'1.0',
'1.0.0',
Expand All @@ -99,7 +99,7 @@ registry`;
const hello = result['hello'];
assert.isDefined(hello);
assert.isDefined(hello.digest);
assert.deepEqual(hello.publishedVersions, [
assert.deepEqual(hello.publishedTags, [
'1',
'1.0',
'1.0.0',
Expand All @@ -123,8 +123,8 @@ registry`;
assert.isTrue(success);
assert.isDefined(infoTagsResult);
const tags = JSON.parse(infoTagsResult.stdout);
const publishedVersions: string[] = tags['publishedVersions'];
assert.equal(publishedVersions.length, 4);
const publishedTags: string[] = tags['publishedTags'];
assert.equal(publishedTags.length, 4);

success = false; // Reset success flag.
try {
Expand Down Expand Up @@ -172,15 +172,15 @@ registry`;
assert.isObject(color);
// Check that the color object has no properties
assert.isUndefined(color.digest);
assert.isUndefined(color.publishedVersions);
assert.isUndefined(color.publishedTags);
assert.isUndefined(color.version);

// -- The breakfix version of hello was updated, so major and minor should be published again, too.
const hello = result['hello'];
assert.isDefined(hello);
assert.isDefined(hello.digest);
assert.isArray(hello.publishedVersions);
assert.deepEqual(hello.publishedVersions, [
assert.isArray(hello.publishedTags);
assert.deepEqual(hello.publishedTags, [
'1',
'1.0',
'1.0.1',
Expand Down Expand Up @@ -219,7 +219,7 @@ registry`;
const newColor = result['new-color'];
assert.isDefined(newColor);
assert.isDefined(newColor.digest);
assert.deepEqual(newColor.publishedVersions, [
assert.deepEqual(newColor.publishedTags, [
'1',
'1.0',
'1.0.1',
Expand All @@ -234,7 +234,7 @@ registry`;
const hello = result['hello'];
assert.isDefined(hello);
assert.isDefined(hello.digest);
assert.deepEqual(hello.publishedVersions, [
assert.deepEqual(hello.publishedTags, [
'1',
'1.0',
'1.0.0',
Expand Down Expand Up @@ -299,8 +299,8 @@ registry`;
assert.isTrue(success);
assert.isDefined(infoTagsResult);
const tags = JSON.parse(infoTagsResult.stdout);
const publishedVersions: string[] = tags['publishedVersions'];
assert.equal(publishedVersions.length, 4);
const publishedTags: string[] = tags['publishedTags'];
assert.equal(publishedTags.length, 4);
});
});

Expand Down
Loading

0 comments on commit 88cbc17

Please sign in to comment.