Refactor the deprecated APIs

[mpanchajanya]

What this PR does / why we need it

• Remove Deprecated APIs usage like StoreClientConfig.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

• Test Setting, Getting feature flags

• Set, Get features and env

mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz version
version: v1.2.0-dev
buildDate: 2024-01-19
sha: 46cba354
arch: arm64
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz config get
clientOptions:
    features:
        global:
            context-target-v2: "true"
cli:
    ceipOptIn: "true"
    eulaStatus: accepted
    discoverySources:
        - oci:
            name: default
            image: projects.registry.vmware.com/tanzu_cli/plugins/plugin-inventory:latest
    cliId: 50314dbb-739f-46b1-8062-a68ba082ea1b
    telemetry:
        source: /Users/mpanchajanya/.config/tanzu-cli-telemetry/cli_metrics.db
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz config set
[x] : both PATH and <value> are required
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis) [1]> tz config set features
[x] : both PATH and <value> are required
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis) [1]> tz config set features.test test
[x] : unable to parse config path parameter into three parts [features.test]  (was expecting 'features.<plugin>.<feature>'
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis) [1]> tz config set features.global.foo true
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz config get
clientOptions:
    features:
        global:
            context-target-v2: "true"
            foo: "true"
cli:
    ceipOptIn: "true"
    eulaStatus: accepted
    discoverySources:
        - oci:
            name: default
            image: projects.registry.vmware.com/tanzu_cli/plugins/plugin-inventory:latest
    cliId: 50314dbb-739f-46b1-8062-a68ba082ea1b
    telemetry:
        source: /Users/mpanchajanya/.config/tanzu-cli-telemetry/cli_metrics.db
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz config set env
[x] : both PATH and <value> are required
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis) [1]> tz config set env.foo bar
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> tz config get
clientOptions:
    features:
        global:
            context-target-v2: "true"
            foo: "true"
    env:
        foo: bar
cli:
    ceipOptIn: "true"
    eulaStatus: accepted
    discoverySources:
        - oci:
            name: default
            image: projects.registry.vmware.com/tanzu_cli/plugins/plugin-inventory:latest
    cliId: 50314dbb-739f-46b1-8062-a68ba082ea1b
    telemetry:
        source: /Users/mpanchajanya/.config/tanzu-cli-telemetry/cli_metrics.db
mpanchajanya@mpanchajanF09MK ~/v/r/tanzu-cli (implement_feature_flags_apis)> 

Release note

Remove usage of deprecated Configuration APIs like StoreClientConfig

Additional information

Special notes for your reviewer

[marckhouzam]

Some comments in-line.

Also, I think you can use configlib.GetAllFeatureFlags() in the function completionGetEnvAndFeatures() instead of accessing the content of the config struct directly.

Finally, I still see one more use of these functions in pkg/interfaces/config_client_interface.go, but considering the time, we can start with this PR.

[marckhouzam]

Why are we doing this now? I don't see it being done before this PR

[mpanchajanya]

@marc pkg/config/In init.go we were already doing this sync of contexts and servers
// contexts could be lost when older plugins edit the config, so populate them from servers
addedContexts := config.PopulateContexts(c)

I removed the use of StoreClientConfig and used the Config APIs directly.

[marckhouzam]

Ok, now I understand two things:

1. StoreClientConfig() would of course store the config, which config.PopulateContexts(c) does not do, so in this PR you save each context in the config file by looping and calling config.SetContext()
2. StoreClientConfig() also made sure any missing servers were created based on the contexts, and now the loop achieves the same thing by the same calls to config.SetContext()

This is great, but could you add a comment before the loop to say something like // Now write the context to the configuration file. This will also create any missing server for its corresponding context

Also, I think there still one aspect missing:

• config.PopulateContexts(c) might have change the value of the currentContext; this is not persisted in the config file because the loop never sets the currentContext

I was able to reproduce that last point as follows:

1. manually delete a context from ~/.config/tanzu/config-ng.yaml
2. set the deleted context as the current in ~/.config/tanzu/config.yaml
3. => So now we have a server that is active that does not exist as a context.
4. Now run tz context list (or any other command).
5. => before this PR, the missing context would be added and be set to active, but with this PR the missing context is added but is not set to active.

[mpanchajanya]

Thanks for the detailed review. I have added comments for SetContext and updated the code to use SetActiveContext.

Started a new review

[marckhouzam]

One last comment and then I think we are good.

[marckhouzam]

LGTM
Thanks for this cleanup!