Skip to content

Add a new command for annotating image digests for EOL#1334

Merged
NikolaMilosavljevic merged 15 commits intodotnet:mainfrom
NikolaMilosavljevic:annotate.digests.command
Jun 19, 2024
Merged

Add a new command for annotating image digests for EOL#1334
NikolaMilosavljevic merged 15 commits intodotnet:mainfrom
NikolaMilosavljevic:annotate.digests.command

Conversation

@NikolaMilosavljevic
Copy link
Member

@NikolaMilosavljevic NikolaMilosavljevic commented Jun 13, 2024

Contributes to: #1201

The work is being split into smaller PRs to aid in review process.

Some notes:

  • We allow two scenarios for annotations - with and without checking for existing annotations.
  • Check for existing annotations relies on oras discover command and its stdout. This will likely be updated once Oras completed the work on managed library.
  • oras login command was not needed, as we already do docker login
  • We do not fail if we fail to annotate one digest, we proceed until the end and log all failures, at which point we fail the command.
  • There are 3 tests right now. We could add more later.

JSON file format (with sample dates):

{
  "eolDate": "2024-05-27",
  "eolDigests": [
    {
      "digest": "digest-value"
    },
    {
      "digest": "digest-value",
      "eolDate" : "2022-03-01"
    }
  ]
}

EOL data can be specified as a global value (optional) for all digests in the file. Each digest can have its own specific EOL date, which overrides the global value (if set).

@NikolaMilosavljevic NikolaMilosavljevic requested a review from a team as a code owner June 13, 2024 20:08
@ghost
Copy link

ghost commented Jun 13, 2024

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged label Jun 13, 2024
@ghost
Copy link

ghost commented Jun 13, 2024

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@NikolaMilosavljevic
Copy link
Member Author

Hmm, some test failures on Linux. This worked fine on Windows.

Working on a fix.

Comment on lines 33 to 34
CreateOption<bool>("no-check", nameof(AnnotateEolDigestsOptions.NoCheck),
"Annotate always, without checking if digest is already annotated for EOL"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using force as the name of the option instead. It's more commonly used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

force was the option I also considered. Yeah, it makes sense to use it - will fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 12fe0a4

}
}
}
#nullable restore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned from @lbussell that these #nullable directives at the end of the file aren't needed. The scope of #nullable enable only applies to the file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 12fe0a4

Comment on lines 21 to 23
public AnnotateEolDigestsOptions() : base()
{
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, forgot to remove - thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 12fe0a4

NikolaMilosavljevic and others added 4 commits June 14, 2024 08:29
…stsOptions.cs

Co-authored-by: Matt Thalman <mthalman@microsoft.com>
Co-authored-by: Matt Thalman <mthalman@microsoft.com>
…stsCommand.cs

Co-authored-by: Matt Thalman <mthalman@microsoft.com>
@NikolaMilosavljevic NikolaMilosavljevic merged commit e4a5db1 into dotnet:main Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants