Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate oc secrets subcomands #18093

Merged
merged 2 commits into from
Jan 16, 2018

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Jan 12, 2018

While recently working on the secrets issue I've noticed we have a set of oc secrets subcommands that can be easily replaced with oc create secret command. Thus, after discussing this topic with @juanvallejo and @deads2k I'm deprecating them.

/assign @juanvallejo @deads2k

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 12, 2018
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 12, 2018
@mfojtik
Copy link
Contributor

mfojtik commented Jan 12, 2018

LGTM

You will need to update the release notes for 3.9 and mark them as deprecated and being removed in 3.10. The release note issue should be in openshift-docs.

@soltysh
Copy link
Contributor Author

soltysh commented Jan 12, 2018

You will need to update the release notes for 3.9 and mark them as deprecated and being removed in 3.10. The release note issue should be in openshift-docs.

Once this merges I'll update the docs. We are using this in several places and that needs to be changed as well.

@soltysh
Copy link
Contributor Author

soltysh commented Jan 12, 2018

/retest

@@ -44,11 +44,6 @@ func NewCmdSecrets(name, fullName string, f *clientcmd.Factory, reader io.Reader
Run: cmdutil.DefaultSubCommandRun(errOut),
}

newSecretFullName := fullName + " " + NewSecretRecommendedCommandName
cmds.AddCommand(NewCmdCreateSecret(NewSecretRecommendedCommandName, newSecretFullName, f, out))
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect a deprecation warning, but I still expect to be able to run oc secret create. Is this not doing what I think it's doing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe they are still being added to the oc secrets tree here https://github.com/openshift/origin/pull/18093/files#diff-98f0c62da95151f788ac0edc23423802R200, but with the deprecation warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you still can run those, it'll just scream after you with the deprecation.

@@ -69,6 +69,9 @@ func NewCmdCreateBasicAuthSecret(name, fullName string, f kcmdutil.Factory, read
Short: "Create a new secret for basic authentication",
Long: createBasicAuthSecretLong,
Example: fmt.Sprintf(createBasicAuthSecretExample, fullName, newSecretFullName, ocEditFullName),
PreRun: func(cmd *cobra.Command, args []string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to change this to Deprecated variable in cobra.Command.

@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 16, 2018
@soltysh
Copy link
Contributor Author

soltysh commented Jan 16, 2018

This is ready to go, @deads2k @juanvallejo make the honors

@deads2k
Copy link
Contributor

deads2k commented Jan 16, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, soltysh

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@soltysh
Copy link
Contributor Author

soltysh commented Jan 16, 2018

/retest

@juanvallejo
Copy link
Contributor

/test extended_conformance_install

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 17976, 17195, 18093, 18080, 17922).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants