MGMT-16002: Accept yml.patch and yaml.patch extensions #6070

paul-maidment · 2024-03-10T17:06:58Z

Presently we do not accept filenames containing yml.patch or yaml.patch
We should accept these files.
This PR rectifies that.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
Performed curl requests against an assisted-test-infra based cluster that is running this service image. yml.patch and yaml.patch files were accepted as expected.
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2024-03-10T17:07:02Z

codecov · 2024-03-10T17:29:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.58%. Comparing base (2f7c5ca) to head (9546333).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6070      +/-   ##
==========================================
+ Coverage   68.36%   69.58%   +1.22%     
==========================================
  Files         239      240       +1     
  Lines       35529    37517    +1988     
==========================================
+ Hits        24291    26108    +1817     
- Misses       9131     9197      +66     
- Partials     2107     2212     +105

Files	Coverage Δ
internal/manifests/manifests.go	`78.49% <100.00%> (+0.83%)`	⬆️

... and 12 files with indirect coverage changes

gamli75 · 2024-03-13T19:58:20Z

data/default_release_images.json

@@ -92,17 +92,17 @@
        "cpu_architectures": [
            "x86_64"
        ],
-        "url": "quay.io/openshift-release-dev/ocp-release:4.13.35-x86_64",
-        "version": "4.13.35"
+        "url": "quay.io/openshift-release-dev/ocp-release:4.13.36-x86_64",


all changes in this file are not related to your PR - please remove it

deploy/podman/configmap.yml

openshift/template.yaml

gamli75 · 2024-03-17T08:26:05Z

@adriengentil please review

adriengentil · 2024-03-18T08:52:41Z

@paul-maidment can you add a word about the patch extension in the doc? https://github.com/openshift/assisted-service/blob/master/docs/user-guide/install-customization.md?plain=1#L9

internal/manifests/manifests.go

adriengentil · 2024-03-18T09:04:57Z

internal/manifests/manifests.go

@@ -540,12 +540,12 @@ func (m *Manifests) validateUserSuppliedManifest(ctx context.Context, clusterID
 		if !json.Valid(manifestContent) {
 			return m.prepareAndLogError(ctx, http.StatusBadRequest, errors.Errorf("Manifest content of file %s for cluster ID %s has an illegal JSON format", fileName, string(clusterID)))
 		}
-	} else if strings.HasPrefix(extension, ".patch") && (strings.Contains(fileName, ".yaml.patch") || strings.Contains(fileName, ".yml.patch")) {
+	} else if strings.Contains(fileName, ".yaml.patch") || strings.Contains(fileName, ".yml.patch") {
 		if err := isValidYaml(manifestContent); err != nil {


shouldn't we validate that the file is a valid yaml patch instead? https://github.com/openshift/assisted-service/blob/master/internal/common/common.go#L482

Agreed!
I have changed this.

Presently we do not accept filenames containing yml.patch or yaml.patch We should accept these files. This PR rectifies that.

gamli75 · 2024-03-20T19:41:03Z

/retest-required

swagger.yaml

Presently we do not accept filenames containing *yml.patch* or *yaml.patch* We should accept these files. This PR rectifies that.

openshift-ci · 2024-03-21T10:56:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adriengentil, paul-maidment

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adriengentil,paul-maidment]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2024-03-21T11:29:04Z

/retest-required

Remaining retests: 0 against base HEAD 2f7c5ca and 2 for PR HEAD 9546333 in total

adriengentil · 2024-03-21T14:34:12Z

/retest

openshift-ci-robot · 2024-03-21T16:43:02Z

/retest-required

Remaining retests: 0 against base HEAD 41ad865 and 1 for PR HEAD 9546333 in total

adriengentil · 2024-03-22T08:41:57Z

/retest

openshift-ci · 2024-03-22T12:41:14Z

@paul-maidment: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-e2e-ai-operator-disconnected-capi	`9546333`	link	false	`/test edge-e2e-ai-operator-disconnected-capi`
ci/prow/edge-e2e-ai-operator-ztp-capi	`9546333`	link	false	`/test edge-e2e-ai-operator-ztp-capi`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 10, 2024

openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 10, 2024

openshift-ci bot requested review from adriengentil and romfreiman March 10, 2024 17:07

openshift-ci bot added api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 10, 2024

paul-maidment force-pushed the MGMT-16002 branch from 4fc2e0e to 82373ac Compare March 11, 2024 08:54

gamli75 reviewed Mar 13, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2024

adriengentil reviewed Mar 18, 2024

View reviewed changes

MGMT-16002: Allow *yaml.patch* and *yml.patch* filenames for manifests

04f1179

Presently we do not accept filenames containing yml.patch or yaml.patch We should accept these files. This PR rectifies that.

paul-maidment force-pushed the MGMT-16002 branch from 82373ac to 48f3be2 Compare March 20, 2024 14:09

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2024

paul-maidment force-pushed the MGMT-16002 branch from 48f3be2 to 0291faf Compare March 20, 2024 15:10

paul-maidment force-pushed the MGMT-16002 branch from 0291faf to 7827894 Compare March 21, 2024 09:55

adriengentil reviewed Mar 21, 2024

View reviewed changes

swagger.yaml Show resolved Hide resolved

paul-maidment force-pushed the MGMT-16002 branch from 7827894 to dd40439 Compare March 21, 2024 10:07

MGMT-16002: Accept *yml.patch* and *yaml.patch* extensions

9546333

Presently we do not accept filenames containing *yml.patch* or *yaml.patch* We should accept these files. This PR rectifies that.

paul-maidment force-pushed the MGMT-16002 branch from dd40439 to 9546333 Compare March 21, 2024 10:52

adriengentil approved these changes Mar 21, 2024

View reviewed changes

openshift-ci bot assigned adriengentil Mar 21, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 21, 2024

openshift-merge-bot bot merged commit 9ad7c61 into openshift:master Mar 22, 2024
14 of 16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-16002: Accept yml.patch and yaml.patch extensions #6070

MGMT-16002: Accept yml.patch and yaml.patch extensions #6070

paul-maidment commented Mar 10, 2024

openshift-ci-robot commented Mar 10, 2024 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

codecov bot commented Mar 10, 2024 •

edited

gamli75 Mar 13, 2024

gamli75 commented Mar 17, 2024

adriengentil commented Mar 18, 2024

adriengentil Mar 18, 2024

paul-maidment Mar 20, 2024

gamli75 commented Mar 20, 2024

openshift-ci bot commented Mar 21, 2024

openshift-ci-robot commented Mar 21, 2024

adriengentil commented Mar 21, 2024

openshift-ci-robot commented Mar 21, 2024

adriengentil commented Mar 22, 2024

openshift-ci bot commented Mar 22, 2024

MGMT-16002: Accept *yml.patch* and *yaml.patch* extensions #6070

MGMT-16002: Accept *yml.patch* and *yaml.patch* extensions #6070

Conversation

paul-maidment commented Mar 10, 2024

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Mar 10, 2024 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

codecov bot commented Mar 10, 2024 • edited

Codecov Report

gamli75 Mar 13, 2024

Choose a reason for hiding this comment

gamli75 commented Mar 17, 2024

adriengentil commented Mar 18, 2024

adriengentil Mar 18, 2024

Choose a reason for hiding this comment

paul-maidment Mar 20, 2024

Choose a reason for hiding this comment

gamli75 commented Mar 20, 2024

openshift-ci bot commented Mar 21, 2024

openshift-ci-robot commented Mar 21, 2024

adriengentil commented Mar 21, 2024

openshift-ci-robot commented Mar 21, 2024

adriengentil commented Mar 22, 2024

openshift-ci bot commented Mar 22, 2024

MGMT-16002: Accept yml.patch and yaml.patch extensions #6070

MGMT-16002: Accept yml.patch and yaml.patch extensions #6070

openshift-ci-robot commented Mar 10, 2024 •

edited by openshift-ci bot

codecov bot commented Mar 10, 2024 •

edited