Do not save component name in env.yaml

[cdrage]

What type of PR is this:

/kind bug

What does this PR do / why we need it:

By default we should be saving the namespace / project name, not the app name /
component name.

Which issue(s) this PR fixes:

Fixes #5780
Fixes #5886

PR acceptance criteria:

• Unit test

• Integration test

• Documentation

How to test changes / Special notes to the reviewer:

Signed-off-by: Charlie Drage charlie@charliedrage.com

[netlify]

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	1ee8de7
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/62e18526bbc88700083c3245

[cdrage]

/retest

[feloy]

I think you will have to make the change on deploy command also (pkg/odo/cli/deploy/deploy.go)

[feloy]

Renaming the PR because of #5663

[odo-robot]

Unit Tests on commit 7ac3c60 finished successfully.
View logs: TXT HTML

[odo-robot]

Kubernetes Tests on commit 7ac3c60 finished successfully.
View logs: TXT HTML

[odo-robot]

OpenShift Tests on commit 7ac3c60 finished successfully.
View logs: TXT HTML

[odo-robot]

Windows Tests (OCP) on commit 7ac3c60 finished successfully.
View logs: TXT HTML

[odo-robot]

Validate Tests on commit 7ac3c60 finished successfully.
View logs: TXT HTML

[valaparthvi]

@cdrage can you also take #5886 into account while working on this PR? Philippe mentioned your changes should fix the issue, and I was wondering if you could add a test if required.

[cdrage]

Thanks @valaparthvi I'll cover a test.

The problem is that I am going on PTO shortly so I won't be able to get back to this for another week.

I'm also running into this issue: #5927 which is not helping with regards to diagnosing what integration test is failing. So I've been working on that so I can test these changes locally.

Unfortunately the IBM test cloud isn't showing the logging (see K8s slack) for which test is failing as the log (I presume) is too large to show.

I'll let you know what progress I get with this, but this is my current update!

[openshift-ci]

@cdrage: PR needs rebase.

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.

[sonarqubecloud]

Please retry analysis of this Pull-Request directly on SonarCloud.

[cdrage]

Rebasing and re-pushing, but up for another review! 👍

[rm3l]

LGTM feature-wise, but would it be possible to clean up the code by removing both Name and AppName fields from the ComponentSettings struct?
I guess this will require cleaning up further things, like removing the GetApplication and GetName methods from the LocalConfigProvider interface as well..

Maybe I've missed something, but I think that this way, we would be sure no one can use that information (I've tested this PR against an existing project that already has component and app names in its env.yaml files, but then from the code perspective, there is currently nothing in this PR preventing from loading and using such information).

[cdrage]

LGTM feature-wise, but would it be possible to clean up the code by removing both Name and AppName fields from the ComponentSettings struct? I guess this will require cleaning up further things, like removing the GetApplication and GetName methods from the LocalConfigProvider interface as well..

Maybe I've missed something, but I think that this way, we would be sure no one can use that information (I've tested this PR against an existing project that already has component and app names in its env.yaml files, but then from the code perspective, there is currently nothing in this PR preventing from loading and using such information).

Let's leave this for now for a future refactor as that'd have to include a lot of changes to LocalConfigProvider and ComponentSEttings.

[openshift-ci]

New changes are detected. LGTM label has been removed.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
1.3% Duplication

[openshift-ci]

@cdrage: The following test 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/unit	40f6804	link	true	/test unit

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.

[rm3l]

LGTM feature-wise, but would it be possible to clean up the code by removing both Name and AppName fields from the ComponentSettings struct? I guess this will require cleaning up further things, like removing the GetApplication and GetName methods from the LocalConfigProvider interface as well..
Maybe I've missed something, but I think that this way, we would be sure no one can use that information (I've tested this PR against an existing project that already has component and app names in its env.yaml files, but then from the code perspective, there is currently nothing in this PR preventing from loading and using such information).

Let's leave this for now for a future refactor as that'd have to include a lot of changes to LocalConfigProvider and ComponentSEttings.

Alright - let's track this in a separate issue then.

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

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 [rm3l]

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

[valaparthvi]

/override ci/prow/v4.10-integration-e2e

[openshift-ci]

@valaparthvi: Overrode contexts on behalf of valaparthvi: ci/prow/v4.10-integration-e2e

In response to this:

/override ci/prow/v4.10-integration-e2e

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.

[cdrage]

Re-added LGTM due to rebase