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

Remove deprecated -ttl flag from spire server cli #5483

Merged
merged 5 commits into from
Sep 12, 2024

Conversation

heymarcel
Copy link
Contributor

This commit removes the deprecated -ttl flag from spire entry create and spire entry update.
See discussion in issue #5254.

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality
This commit removes the deprecated -ttl flag from spire entry create and spire entry update.

Description of change
As per the issue referenced above:

The CLI has long since deprecated the -ttl flag in favor of the SVID specific flags -x509SVIDTTL and -jwtSVIDTTL. The deprecated flag should be removed.

Which issue this PR fixes
fixes#5254

This commit removes the deprecated `-ttl` flag from `spire entry
create` and `spire entry update`. Docs are also updated.

See discussion in spiffe#5254

Signed-off-by: Marcel Levy <marcel@spirl.com>
azdagron
azdagron previously approved these changes Sep 10, 2024
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @heymarcel !

@MarcosDY
Copy link
Collaborator

MarcosDY commented Sep 11, 2024

can you update ITs that are using -ttl and replace that with x509SVIDTTL or jwtSVIDTTL depending the case?
you can found ITs here

@heymarcel
Copy link
Contributor Author

@MarcosDY Sorry about missing the integration tests. I have them working now, but I notice the Windows unit tests are still failing. Is that expected, or am I missing something? https://github.com/heymarcel/spire/actions/runs/10815947896/job/30006064864

@heymarcel heymarcel requested a review from azdagron September 11, 2024 17:50
@MarcosDY
Copy link
Collaborator

MarcosDY commented Sep 11, 2024

Thanks for taking care of this!
Yes, you'll need to update the expected messages for Windows here
Unfortunately, you'll need a Windows environment to test this, but we can rely on CI for that. It should be pretty straightforward.

@heymarcel
Copy link
Contributor Author

Thanks! I've got that running now as well.

@MarcosDY
Copy link
Collaborator

Looks good but, one last thing,
can you resolve DCO?

Signed-off-by: Marcel Levy <marcel@spirl.com>
Signed-off-by: Marcel Levy <marcel@spirl.com>
@amartinezfayo amartinezfayo merged commit 32eaeca into spiffe:main Sep 12, 2024
34 checks passed
@amartinezfayo amartinezfayo added this to the 1.11.0 milestone Sep 12, 2024
@heymarcel heymarcel deleted the ttl-5254 branch September 13, 2024 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants