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.

Sign up for GitHub

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

Jump to bottom

pkg/osbuild: add ExpireDate to users #513

Merged
merged 2 commits into from
Mar 22, 2024
Merged

pkg/osbuild: add ExpireDate to users #513

merged 2 commits into from
Mar 22, 2024

Conversation

andremarianiello
Copy link
Contributor

The ExpireDate field for the org.osbuild.users stage of osbuild has been added in osbuild/osbuild#1648. This PR simply extends osbuild/images to support the new field.

@andremarianiello andremarianiello marked this pull request as ready for review March 13, 2024 01:57
@andremarianiello
Copy link
Contributor Author

If you have any testing recommendations/requirements on this please let me know!

@andremarianiello
Copy link
Contributor Author

I can't view the snyk scan issues, so I need a bit of assistance there. Thanks in advance!

supakeen
supakeen reviewed Mar 13, 2024
View reviewed changes
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

Looks OK but it seems your editor (or you) also made some other changes. I'm fine with the other changes as well but I'd prefer them in two commits.

@andremarianiello
Copy link
Contributor Author

andremarianiello commented Mar 13, 2024 •
edited
Loading

Looks OK but it seems your editor (or you) also made some other changes. I'm fine with the other changes as well but I'd prefer them in two commits.

It was go fmt. I'll separate those out!

@andremarianiello andremarianiello force-pushed the add-user-expiredate branch 2 times, most recently from c2fb2c9 to 8f60c8d Compare March 18, 2024 14:46
@andremarianiello
Copy link
Contributor Author

Looks OK but it seems your editor (or you) also made some other changes. I'm fine with the other changes as well but I'd prefer them in two commits.

I have split it into separate commits.

supakeen
supakeen approved these changes Mar 18, 2024
View reviewed changes
Copy link
Member

@supakeen supakeen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me. Thank you. I can't view the Snyk results either so this is pending the CI :)

@andremarianiello
Copy link
Contributor Author

Thank you! Does anything have to be done about the failing snyk check?

@andremarianiello
Copy link
Contributor Author

This PR has breaking changes with osbuild-composer. I plan on updating that repo as well, but I don't know how I make the checks here pass. Seems like a circular dependency.

@supakeen
Copy link
Member

Reasonably sure that they're to be expected but we might want to have a PR ready in osbuild-composer before this merges, cc @achilleas-k?

@achilleas-k
Copy link
Member

This PR has breaking changes with osbuild-composer. I plan on updating that repo as well, but I don't know how I make the checks here pass. Seems like a circular dependency.

Yeah that's to be expected. It's more of a warning so it's obvious that the update in osbuild-composer will require changes.

@achilleas-k
Copy link
Member

Reasonably sure that they're to be expected but we might want to have a PR ready in osbuild-composer before this merges, cc @achilleas-k?

I think we can merge without the PR being ready since the changes are rather trivial.

@achilleas-k
Copy link
Member

I also don't have access to snyk.

@achilleas-k achilleas-k added this pull request to the merge queue Mar 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 18, 2024
@achilleas-k achilleas-k added this pull request to the merge queue Mar 19, 2024
@achilleas-k
Copy link
Member

Merging failed on the queue because of a quay.io issue. Hoping it's resolved now.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 19, 2024
@achilleas-k achilleas-k force-pushed the add-user-expiredate branch from e569068 to 75c50df Compare March 19, 2024 21:01
@andremarianiello
Copy link
Contributor Author

Looks like some of the gitlab CI jobs failed, probably due to issues on the runner?

@bcl bcl force-pushed the add-user-expiredate branch from 75c50df to a5beebe Compare March 21, 2024 00:21
@andremarianiello
Copy link
Contributor Author

Worth trying merge queue again?

Apologies if my daily comments are too frequent, please let me know if it is too much 🙏 I would do it myself if I could

@bcl bcl force-pushed the add-user-expiredate branch from a5beebe to f31f824 Compare March 21, 2024 21:25
@achilleas-k achilleas-k added this pull request to the merge queue Mar 22, 2024
@achilleas-k
Copy link
Member

achilleas-k commented Mar 22, 2024 •
edited
Loading

Apologies if my daily comments are too frequent

No apologies necessary. It's fine.

Merged via the queue into osbuild:main with commit cce2f14 Mar 22, 2024
13 of 16 checks passed
@andremarianiello
Copy link
Contributor Author

Thank you so much for all you efforts with this! I will have an osbuild-composer PR today

croissanne added a commit to croissanne/osbuild-composer that referenced this pull request Mar 25, 2024
@croissanne
internal/blueprint: sync with images repository
54ed33a 
See osbuild/images#513
@croissanne croissanne mentioned this pull request Mar 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@supakeen supakeen supakeen approved these changes

@achilleas-k achilleas-k achilleas-k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andremarianiello @supakeen @achilleas-k