text:
[Remote Store] Avoiding Translog Deletion in case of relocating shards.

Description
In Primary to Primary relocation , there can be concurrent upload and download of translog.
While translog files are getting downloaded by new primary, it might hence be deleted by the relocating primary
Hence we need to avoid to deletion when shard is in RELOCATING state .
Related Issues
Resolves #9191
Check List

[x] New functionality includes testing.

[x] All tests pass

[x] New functionality has been documented.

[x] New functionality has javadoc added

[ ] Commits are signed per the DCO using --signoff
[ ] Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Failure not related to this change : https://github.com/opensearch-project/OpenSearch/issues/9580


text:
[BUG] a lot of getting all indices error when open sample detector page

What is the bug?
See a lot of "Error getting all indices" error when open sample detector.
How can one reproduce the bug?
Steps to reproduce the behavior:

Create multi node cluster with CDK
Create 3 sample detectors
Open sample-ecommerce-detector
See a lot of Error getting all indices error. Open browser network, see many requests of _indices?index=.

What is the expected behavior?
Just throw one error and don't sent out so many request in short time.
What is your host/environment?

OS: Amazon Linux 2
Version 1.3.1
Plugins

Do you have any screenshots?

Do you have any additional context?
Add any other context about the problem.

we have this problem also, with our regular indices. I have this screenshot from java script console. So it seems that the browser could not process so much requests at once.

More specifically what is happening is that the useFetchDetectorInfo(detectorId) method is getting called in a loop until various conditions are met: https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/main/public/pages/CreateDetectorSteps/hooks/useFetchDetectorInfo.ts#L23. Solution would either be to change useFetchDetectorInfo() to only get called once and wait for response or to add if (!isLoadingDetector) before calling getInitialIndices() so we don't make constant network requests while detector is being fetched.

source: gharchive/issue

authors: amitgalitz inidona ylwu-amzn

license_type: permissive

license_source: github-api


text:
Add an AD transport client

Description
This PR adds an AD transport client for the following transport actions:

search anomaly detectors
search anomaly results

This follows the same approach as the ML commons client - it is constructed using the caller's node client, and uses it to execute various transport actions.
For these particular methods, we don't do any special pre- or post-processing. These actions are mostly simple pass-through actions to query the internal system indices, and take a general SearchRequest and return a general SearchResponse. We do the same with the client methods that are added.
Note that the unit tests are intended to ensure the underlying transport actions of the client methods are being exercised and returning expected results, covering some of the basic use cases (no system index, empty results, populated results). The exhaustive set of transport action scenarios are within the respective transport action test suites themselves. We do not want/need to unnecessarily duplicate all of those tests here, as it would add require more places that would need to be updated, should the action interfaces change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Not backporting. I have a manual branch ad-client I will open against 2.x when this is merged. They need to be maintained separately because of the package refactoring done on main.

@joshpalis CIs are passing on Java 17+ which is what the current codebase is compiled and formatted on. We should be fine to merge and potentially update CIs when minimum Java version is defined for 3.x. Regardless, prefer to leave out any logic changing that as part of this PR which is unrelated.

source: gharchive/pull-request

repo: opensearch-project/anomaly-detection


text:
add ignore_findings_and_alerts field to monitors

Description
add ignore_findings_and_alerts field to monitors
Related Issues
Resolves #[Issue number to be closed when this PR is merged]

Check List

[X] New functionality includes testing.
[ ] New functionality has been documented.
[ ] API changes companion pull request created.
[X] Commits are signed per the DCO using --signoff.
[ ] Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

closing this in favor of #757

repo: opensearch-project/common-utils


text:
[Enhancement] Support serverless compatible index template in opensearch sink

Is your feature request related to a problem? Please describe.
Currently opensearch sink defaults to management_disabled for index management. Since the merge of support for composable index template we have the ability to expand the support for index management for serverless ingestion.
Describe the solution you'd like
Describe alternatives you've considered (Optional)
Additional context

Added to 2.5.0 by backport PR #3467.

repo: opensearch-project/data-prepper


text:
[PATCH RELEASE ] - 1.0.1  patch release

There is an issue in Index Management that breaks backwards compatibility in the response body of individual explain requests. To address this issue there will be a 1.0.1 patch release. This will be tracked in the project roadmap along side the other releases.
If there are any additional bugs that will be patched as part of 1.0.1, please comment on this issue so that it can be incorporated.

For Dashboards:
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/704
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/699
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/698
We also have a set of documentation improvements/fixes that should at least go into our 1.0 branch: https://github.com/opensearch-project/OpenSearch-Dashboards/pulls?q=is%3Apr+is%3Aclosed+label%3Adocs+label%3Av1.1.0

For Dashboards:
opensearch-project/OpenSearch-Dashboards#704
opensearch-project/OpenSearch-Dashboards#699
opensearch-project/OpenSearch-Dashboards#698
We also have a set of documentation improvements/fixes that should at least go into our 1.0 branch: https://github.com/opensearch-project/OpenSearch-Dashboards/pulls?q=is%3Apr+is%3Aclosed+label%3Adocs+label%3Av1.1.0

@tmarkley do you want me to go ahead and add these to the 1.0.1 patch release in the roadmap?

Maybe too late to the party but just wanting to bring an issue with demo certificates in the container images to your attention which IMO will prevent production use of said container images. See #254.

authors: elfisher nebulon42 tmarkley

repo: opensearch-project/opensearch-build


text:
[BUG] Windows integTest stuck on executing integtest.sh commands

Hi, We are currently seeing this issue in Windows Zip, for OS only.

Symptoms:

Running windows integTest on 2.13.0 on a fresh container resulted in these behaviors:

https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test/detail/integ-test/8044/pipeline/107

UnicodeDecodeError: ‘utf-8’ codec can’t decode byte 0xbb in position 523169: invalid start byte

Stuck on 2024-03-23 01:13:49 INFO   Executing "bash C:\Users\ContainerAdministrator\opensearch-build\scripts\default\integtest.sh -b localhost -p 9200 -s true -v 2.13.0" in C:\Users\ContainerAdministrator\tmpwbgpq28i\<plugin name> until timeout

Running windows integTest on 2.12.0 on a fresh container, then run 2.13.0

Test will successfully pass on Windows Zip

Debugging:

Ways to temporary pass the issue, but not persist after a docker container restart:

Run 2.13.0 once,  then run 2.12.0, then run 2.13.0
Run 2.13.0 once, update a print statement anywhere within the src/system/execute.py.
Run 2.13.0 once, update anything related to python code

Changing gradle version does fix the issue
Switch from main to 2.12.0 opensearch-build code does not fix the issue
Ways to permanently pass the issue in POC

Change the integtest.sh execution to something else, like powershell, or use bash to directly call commands without calling a script in the middle

We suspect this is caused by subprocess.run does not receive a proper exit/return from bash integtest.sh, thus keep waiting for the end of the subroutine until time out
Mitigation:

Need some help from the core team / plugin teams to understand if there is any changes related to OS that cause this issue in Windows
Whether gradle tasks has changed that cause this issue
More clues are welcomed because this issue does not happen on 2.12.0 Windows Zip artifacts.

Thanks.

As per subprocess documentation to ignore UnicodeDecodeError exception, we need to add errors=ignore in the subprocess.call command. So I spun up a windows host and ran the windows container.
I ran for opensearch-observability without error=ignore flag, and I got the same UnicodeDecodeError exception , then i add the flag in the command in execute method and it ran successfully. To confirm it works, in the same container session I removed the flag again and got the decode error again and after adding it back it is gone.
I can see the files in the test-results directory.
We can better handle it by catching this exception.
try:
    result = subprocess.run(command, cwd=dir, shell=True, capture_output=capture, text=True, encoding='utf-8')
except UnicodeDecodeError as e:
    print(f"Error decoding stdout: {e}")
    result = subprocess.run(command, cwd=dir, shell=True, capture_output=capture, text=True, encoding='utf-8', errors='ignore')

Thanks Rishabh,
We tried to use the errors already.
The problem is:

It is not clear why sometimes we can reach the unicode error, sometimes it just stuck on executing without even goes to next step.
Even without changing the errors we can still get it pass by running 2.12 before 2.13. Then 2.13 pass.

We will take a look and sync up later.

By switching the gradlew to gradlew.bat on windows, I finally able to consistently reach unicode error step, then I see this error pops up on observability, which also being confirmed that shows up in AD as well:

        Caused by:
        javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
            at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:131)
            at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:360)
            at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:303)
            at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:298)
            at java.base/sun.security.ssl.CertificateMessage$T13CertificateConsumer.checkServerCerts(CertificateMessage.java:1357)
            at java.base/sun.security.ssl.CertificateMessage$T13CertificateConsumer.onConsumeCertificate(CertificateMessage.java:1232)
            at java.base/sun.security.ssl.CertificateMessage$T13CertificateConsumer.consume(CertificateMessage.java:1175)
            at java.base/sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:392)
            at java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:443)
            at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(SSLEngineImpl.java:1076)
            at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction.run(SSLEngineImpl.java:1063)
            at java.base/java.security.AccessController.doPrivileged(Native Method)
            at java.base/sun.security.ssl.SSLEngineImpl$DelegatedTask.run(SSLEngineImpl.java:1010)
            at org.apache.http.nio.reactor.ssl.SSLIOSession.doRunTask(SSLIOSession.java:289)
            at org.apache.http.nio.reactor.ssl.SSLIOSession.doHandshake(SSLIOSession.java:357)
            at org.apache.http.nio.reactor.ssl.SSLIOSession.isAppInputReady(SSLIOSession.java:545)
            at org.apache.http.impl.nio.reactor.AbstractIODispatch.inputReady(AbstractIODispatch.java:120)
            at org.apache.http.impl.nio.reactor.BaseIOReactor.readable(BaseIOReactor.java:162)
            at org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvent(AbstractIOReactor.java:337)
            at org.apache.http.impl.nio.reactor.AbstractIOReactor.processEvents(AbstractIOReactor.java:315)
            at org.apache.http.impl.nio.reactor.AbstractIOReactor.execute(AbstractIOReactor.java:276)
            at org.apache.http.impl.nio.reactor.BaseIOReactor.execute(BaseIOReactor.java:104)
            at org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor$Worker.run(AbstractMultiworkerIOReactor.java:591)
            ... 1 more

            Caused by:
            sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
                at java.base/sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:439)
                at java.base/sun.security.validator.PKIXValidator.engineValidate(PKIXValidator.java:306)
                at java.base/sun.security.validator.Validator.validate(Validator.java:264)
                at java.base/sun.security.ssl.X509TrustManagerImpl.validate(X509TrustManagerImpl.java:313)
                at java.base/sun.security.ssl.X509TrustManagerImpl.checkTrusted(X509TrustManagerImpl.java:276)
                at java.base/sun.security.ssl.X509TrustManagerImpl.checkServerTrusted(X509TrustManagerImpl.java:141)
                at java.base/sun.security.ssl.CertificateMessage$T13CertificateConsumer.checkServerCerts(CertificateMessage.java:1335)
                ... 19 more

                Caused by:
                sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
                    at java.base/sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:148)
                    at java.base/sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:129)
                    at java.base/java.security.cert.CertPathBuilder.build(CertPathBuilder.java:297)
                    at java.base/sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:434)

Added on top you apparently have to manually mount a volume on windows to keep the persistence of its Windows container storage.
I am able to constantly reproduce the bug now.

Note that Jenkins also mount the volume without using existing ones.

`docker run -it -d -v "C:\Users\Administrator\opensearch-build:C:\Users\Administrator\opensearch-build" <> cmd.exe

https://docs.python.org/3/library/functions.html#open
'ignore' ignores errors. Note that ignoring encoding errors can lead to data loss.

'replace' causes a replacement marker (such as '?') to be inserted where there is malformed data.

We will add a errors='replace' for src/system/execute.py's subprocess.run().


text:
Make sure the build repo is pulled in docker-build stage

Signed-off-by: Peter Zhu zhujiaxi@amazon.com
Description
Make sure the build repo is pulled in docker-build stage.
Directly use agent node will have consequences where git repo is not pulled.
Running on docker container will force the pull every time.
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #2060 (02c1262) into main (ec6c37c) will increase coverage by 5.89%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##               main     #2060      +/-   ##
=============================================
+ Coverage     94.10%   100.00%   +5.89%
=============================================
  Files           198         6     -192
  Lines          3902       105    -3797
  Branches         29        19      -10
=============================================
- Hits           3672       105    -3567
+ Misses          224         0     -224
+ Partials          6         0       -6

Impacted Files
Coverage Δ

src/build_workflow/builders.py

src/manifests_workflow/component_opensearch_min.py

src/test_workflow/perf_test/perf_test_cluster.py

.../assemble_workflow/bundle_opensearch_dashboards.py

...sts/jenkins/jobs/ArchiveAssembleUpload_Jenkinsfile

.../ci_workflow/ci_check_gradle_properties_version.py

src/manifests/component_manifest.py

tests/jenkins/jobs/Messages_Jenkinsfile

src/build_workflow/build_args.py

src/test_workflow/test_args.py

... and 182 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec6c37c...02c1262. Read the comment docs.

See OS build 2626 vs 2629 for docker build trigger comparison.

authors: codecov-commenter peterzhuamazon


text:
Implement LocalTestCluster

Description
Implements LocalTestCluster that downloads, unpacks, and starts a local OpenSearch node (via opensearch-tar-install.sh), and tears it down again when the tests are finished.
Testing
Manual testing. I was able to run the SQL plugin integ tests.
Issues Resolved
https://github.com/opensearch-project/opensearch-build/issues/205
Check List

[X] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

If the caller forgets to call destroy, you'll leave everything behind, consider adopting a pattern with yield ala https://github.com/opensearch-project/opensearch-build/blob/948ace083bf8e5fc017db062d1104575f4186327/bundle-workflow/python/system/temporary_directory.py and get rid of try/finally.

Good idea. I'll learn some more Python and address that in a later PR.


text:
Enable Integration tests

We have updated fixed and enabled integrations for Opensearch version >= 1.1.0. We plan on working on 1.0.x in accordance with this issue mentioned here: https://github.com/opensearch-project/project-website/issues/737

This is completed as part of #36 being merged

repo: opensearch-project/opensearch-net


text:
Update jest-cli to 29.7.0

Description

Bump jest and related dependencies to latest major version
Update jsdom
Update snapshots
Skip some focus-related test cases which don't work well in Enzyme/Jsdom

Issues Resolved

Fixes https://github.com/opensearch-project/oui/issues/630
Check List

[ ] New functionality includes testing.
[ ] New functionality has been documented.
[ ] All tests pass

[ ] yarn lint
[ ] yarn test-unit

[ ] Update CHANGELOG.md
[ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Setting to draft in favor of https://github.com/opensearch-project/oui/pull/1102

repo: opensearch-project/oui


text:
[BUG] Sigma Null Filters Not Translated To Query Properly

What is the bug?
Null filters in Sigma rules are not translated properly to queries leading to False Positives and broken detections.
Example:
Pre-packaged rule Wmiprvse Spawning Process contains a null filter

By sigma syntax this should only hit on documents where the LogonId field exists (As in NOT null)
The query is translated to this by OpenSearch:
((winlog.event_data.ParentImage: *\\WmiPrvSe.exe) AND ((NOT (((winlog.event_data.LogonId: "0x3e7") OR (winlog.event_data.LogonId: "null")) OR ((winlog.user.name: *AUTHORI*) OR (winlog.user.name: *AUTORI*)) OR ((winlog.event_data.Image: *\\WmiPrvSE.exe) OR (winlog.event_data.Image: *\\WerFault.exe)))))) AND ((NOT winlog.event_data.LogonId: null))
This is not working as intended. The NOT winlog.event_data.LogonId: null section is actually looking for null as a string.  This query will return all documents that dont have winlog.event_data.LogonId equal to the string "null"
How can one reproduce the bug?
Steps to reproduce the behavior:

Create any Sigma rule that uses a null filter (or use the prepackaged rule noted above)
View its query and note the null filter is translated to NOT : null
Go to discover
Query for NOT <field>:null or simply: NOT winlog.event_data.LogonId:null
All documents where the specified field is not the string "null" are returned (Nearly all data)

What is the expected behavior?
The sigma to be properly translated to something along the lines of:
winlog.event_data.LogonId:* where this returns results where the field being queried exists in the data
What is your host/environment?

OS: 2.11 fresh install

Thanks,
Jay

Resolved by #722

authors: engechas mimicbox

repo: opensearch-project/security-analytics


text:
[FEATURE] Add yarn lint as a pre-commit hook

Is your feature request related to a problem?
Several times when pushing up work, I forget to run yarn lint:es --fix, which leads to tests failing. We should add a hook to automatically lint commits and avoid unnecessary workflow runs.
What solution would you like?
Commits are automatically linted and fixed if possible.
What alternatives have you considered?
None
Do you have any additional context?
We can use husky

[Triage] we will make/accept a contribution for this change


text:
[BUG] Not able to nevigate out of "Application Not Found" page

Describe the bug
Application Not Found page does not have any button or hyperlink for user to leave it or reset the URL
To Reproduce
Steps to reproduce the behavior:

Login as admin
Go to http://localhost:5601/app/qqqqqq
See Application Not Found
No way to leave unless manually pasting http://localhost:5601 to the URL

Expected behavior
Some manual option is provided for the user to leave this page
Plugins
security - FE and BE
Screenshots

Host/Environment (please complete the following information):

OS: [e.g. iOS]
Version [e.g. 22]

Additional context
Add any other context about the problem here.

Apologies for the very late reply. As several versions (including major ones) have come out and we have not heard of reports like this, I'm going to go ahead and close as potentially resolved, please re-open with more details if that is not the case. Thanks!


text:
Disable codecov from failing CI if there is an upload issue

Description
Seeing a ton of CI failures due to code coverage upload failures.  I'd like to fix this - but I'd rather keep the builds flowing in until we have a better solution.
E.g: https://github.com/opensearch-project/security/actions/runs/6153593099/job/16697726519?pr=3339

Related https://github.com/opensearch-project/security/issues/2649

Check List

[ ] New functionality includes testing
[ ] New functionality has been documented
[X] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Retrying tests, network error on windows hosts, case of the network issue we've been tracking [1].

[1] https://github.com/opensearch-project/security/issues/3266

repo: opensearch-project/security


text:
[FEATURE] [META] Querqy Plugin Release

Release  the Querqy Plugin for OpenSearch

Comes in from a community contribution here

More issues to standardize the plugin before release are added here

NOTE: This meta will be moved to a new plugin repository soon.

@ps48 Should we transfer this to https://github.com/opensearch-project/search-relevance/issues ?

repo: opensearch-project/sql


text:
Update Spring to 5.3.25 to cover CVE-2023-20863 "spring-expression DDOS"

Description
Cover CVE-2023-20863 https://www.mend.io/vulnerability-database/CVE-2023-20863
Issues Resolved
Check List

[ ] New functionality includes testing.

[ ] All tests pass, including unit test, integration test and doctest

[ ] New functionality has been documented.

[ ] New functionality has javadoc added
[ ] New functionality has user manual doc added

[x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #1619 (7163619) into 1.3 (0c2ba70) will not change coverage.
The diff coverage is n/a.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
@@            Coverage Diff            @@
##                1.3    #1619   +/-   ##
=========================================
  Coverage     98.40%   98.40%
  Complexity     2746     2746
=========================================
  Files           266      266
  Lines          6758     6758
  Branches        427      427
=========================================
  Hits           6650     6650
  Misses          107      107
  Partials          1        1

Flag
Coverage Δ

sql-engine
98.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

authors: codecov-commenter pjfitzgibbons


text:
Refer Antlr file using specific branch

Description

Refer Antlr file using specific tag/branch (Spark -> v3.5.1, Flint -> 0.5)

Related Issues
https://github.com/opensearch-project/sql/pull/2890#issuecomment-2264199142
Check List

[ ] New functionality includes testing.
[ ] New functionality has been documented.
[ ] New functionality has javadoc added.
[ ] New functionality has a user manual doc added.
[ ] API changes companion pull request created.
[x] Commits are signed per the DCO using --signoff.
[ ] Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

I'm thinking how we can ensure that we remember to update this version whenever a new release branch is created in opensearch-spark repo. cc: @penghuo

Should we have a check list for creating new release branch in opensearch-spark repo?

I'm thinking whether tracking one Flint Spark branch will be a problem or not. I think the ideal case SQL main tracks Spark main and SQL 2.x tracks Spark 0.5? However, we cannot do this because we always backport PR?

I think the problem is that we tend to automatically include the grammar change in the PRs even if the change is not related to the grammar. We'd want to separate Grammar PRs and don't backport them.
Options came up to my mind are:

Do not automatically update Grammar files.
Separate the grammar files for repository and ones for build, and only update the ones for build during build. And add build target to update grammar file for repository.
Manually avoid including the Grammar files to PRs.


text:
(feat) add initContainer to osm-controller

The initContainer creates a MeshConfig resource in k8s if it does not
exist
Signed-off-by: Allen Leigh allenlsy@gmail.com

Description:
This PR is the continuation from #3222.
The initial PR failed the CI test. The cause is that, the MeshConfig CRD spelled the field prometheusScrapping, while the Go struct spelled the field prometheusScraping.

Affected area:

New Functionality      [x]
Install                [x]
Control Plane          [x]

Please answer the following questions with yes/no.

Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?

No

Codecov Report

Merging #3250 (24df303) into main (6681538) will decrease coverage by 0.10%.
The diff coverage is 60.71%.

@@            Coverage Diff             @@
##             main    #3250      +/-   ##
==========================================
- Coverage   66.42%   66.31%   -0.11%
==========================================
  Files         163      164       +1
  Lines        7490     7617     +127
==========================================
+ Hits         4975     5051      +76
- Misses       2488     2539      +51
  Partials       27       27

Flag
Coverage Δ

unittests
66.31% <60.71%> (-0.11%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

cmd/init-osm-controller/init-osm-controller.go
60.71% <60.71%> (ø)

pkg/configurator/validating_webhook.go
71.89% <0.00%> (-9.16%)
:arrow_down:

pkg/catalog/outbound_traffic_policies.go
82.75% <0.00%> (-1.40%)
:arrow_down:

pkg/smi/client.go
78.67% <0.00%> (-1.33%)
:arrow_down:

pkg/smi/fake.go
0.00% <0.00%> (ø)

pkg/envoy/ads/stream.go
5.88% <0.00%> (ø)

pkg/smi/mock_meshspec_generated.go
0.00% <0.00%> (ø)

pkg/envoy/route/route_config.go
95.07% <0.00%> (+0.07%)
:arrow_up:

pkg/catalog/xds_certificates.go
90.12% <0.00%> (+0.12%)
:arrow_up:

... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6681538...24df303. Read the comment docs.

authors: allenlsy codecov-commenter

repo: openservicemesh/osm


text:
(k8s/informers): Use InformerCollection for other clients

Use the new InformerCollection instead of fragmenting informers across
different clients
Signed-off-by: Keith Mattix II keithmattix2@gmail.com

Description:

Testing done:

Affected area:

Functional Area

New Functionality
[ ]

CI System
[ ]

CLI Tool
[ ]

Certificate Management
[ ]

Control Plane
[ ]

Demo
[ ]

Documentation
[ ]

Egress
[ ]

Ingress
[ ]

Install
[ ]

Networking
[ ]

Observability
[ ]

Performance
[ ]

SMI Policy
[ ]

Security
[ ]

Sidecar Injection
[ ]

Tests
[ ]

Upgrade
[ ]

Other
[ ]

Please answer the following questions with yes/no.

Does this change contain code from or inspired by another project?

Did you notify the maintainers and provide attribution?

Is this a breaking change?

Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?

Codecov Report

Merging #4804 (23690d4) into main (47c06ab) will decrease coverage by 0.09%.
The diff coverage is 71.69%.

@@            Coverage Diff             @@
##             main    #4804      +/-   ##
==========================================
- Coverage   68.54%   68.45%   -0.10%
==========================================
  Files         223      223
  Lines       16225    16055     -170
==========================================
- Hits        11122    10991     -131
+ Misses       5052     5013      -39
  Partials       51       51

Flag
Coverage Δ

unittests
68.45% <71.69%> (-0.10%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

cmd/osm-bootstrap/osm-bootstrap.go
46.50% <0.00%> (-1.17%)
:arrow_down:

cmd/osm-controller/osm-controller.go
13.38% <0.00%> (+0.10%)
:arrow_up:

pkg/smi/types.go
100.00% <ø> (ø)

pkg/smi/client.go
76.44% <87.50%> (+2.79%)
:arrow_up:

pkg/configurator/client.go
95.94% <100.00%> (+6.97%)
:arrow_up:

pkg/policy/client.go
91.75% <100.00%> (+3.83%)
:arrow_up:

pkg/certificate/manager.go
88.28% <0.00%> (-2.46%)
:arrow_down:

pkg/envoy/rbac/policy.go
97.67% <0.00%> (-2.33%)
:arrow_down:

... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47c06ab...23690d4. Read the comment docs.

authors: codecov-commenter keithmattix


text:
[BZ#2034625] Approve InstallPlan for Subscription type policies

Description:

parse managed policies to determine if they contain a Subscription object
for policies that contain a Subscription, create a ManagedClusterView
to get access to the details about the InstallPlan
create a ManagedClusterView to get the latest resource version for the
InstallPlan
create a ManagedClusterAction to approve the InstallPlan only if the
approval is set to Manual
at the end of the upgrade, delete the created multicloud objects
have all the functions dealing with multicloud operator objects in a
separate file: multicloud_util.go
TODO: integration tests

/retest-required

/retest-required

/lgtm

authors: irinamihai nishant-parekh


text:
Reconcile kubelet config

[X] Cover letter (this message)
[X] Add configmap mount into daemonset
[ ] Test coverage

revamp how we handle the configuration of the RTE.
We need to supply to the RTE a configuration file to tell it:

details about the TM configuration, which it cannot figure out otherwise (policy, scope...)
parameters needed by the sysinfo package, which it cannot determine from the system because they depend on the kubelet configuration.

The second set is going to be phased out when OCP exposed the latest changes in the podresources API, but the first set will always be needed. So we will need some form of configuration management anyway.
So the operator will translate the information from the Kubeletconfig object, if available, to the RTE config.yaml and make it available as configmap.
Most important design decisions:
a. always translate as much configuration as possible, since it's cheap once we have access to the relevant kubeconfig. This for example include always reporting reservedCPUs, even when the RTE will no longer need this information (podresources' GetAllocatable available). RTE will just ignore the fields it doesn't need anymore
b. build the configuration only on OpenShift; on Kubernetes, we can just mount the kubelet config on the RTE pod and let it figure out.
In order to process the kubeletconfig, we introduced a new controller.
The new controller will watch the kubeletconfigs and reconcile the relevant configmaps.
We add a new controller to not overload the existing controller, and because, per above design decisions, we can nicely split the responsabilities and make the two reconciliation loops (almost) completely independent: the numaresources main controller will deploy the bigger part of the stack; the kubeletconfig controller will provide configuration, if needed and possible.
(xref: "One controller per custom resource definition" from https://sdk.operatorframework.io/docs/best-practices/best-practices/)
NOTE: we will handle the RTE side (noticing the update configuration) on a separate PR

reason of failure:
Events:
  Type     Reason       Age               From               Message
  ----     ------       ----              ----               -------
  Normal   Scheduled    72s               default-scheduler  Successfully assigned numaresources/numaresourcesoperator-mcp-test-xxkcs to kind-worker
  Warning  FailedMount  9s (x8 over 72s)  kubelet            MountVolume.SetUp failed for volume "rte-config" : configmap "numaresourcesoperator-mcp-test" not found

This is actually good because it fails fast and reliably. It is indeed caused by the pending design decisions :)

note this is fixed (and the fix is debatable) on kube, but it's silently broken on OCP.

/hold
/lgtm
feel free to unhold when you feel comfortable ;)

/hold /lgtm feel free to unhold when you feel comfortable ;)

sure, want to add some tests before

/hold cancel

/lgtm

repo: openshift-kni/numaresources-operator


text:
print out hypershfit cluster credential

It print credentials in the output, which is  useful for debugging

/lgtm

repo: openshift-pipelines/pipeline-service


text:
Fix issue related to repository URL if it contains '/' at the end

Issue
Repository with spec.url ending with '/' is not matching URL coming from Github.
Changes
Removed slashes from the repo.spec.url in order to match with the URL coming from Github

Submitter Checklist

[ ] ♽  Run make test lint before submitting a PR (ie: with pre-commit, no need to waste CPU cycle on CI
[ ] 📖 If you are adding a user facing feature or make a change of the behavior, please verify that you have documented it
[ ] 🧪 100% coverage is not a target but most of the time we would rather have a unit test if you make a code change.
[ ] 🎁 If that's something that is possible to do please ensure to check if we can add a e2e test.
[ ] 🔎 If there is a flakiness in the CI tests then don't necessary ignore it, better get the flakyness fixed before merging or if that's not possible there is a good reason to bypass it. (token rate limitation may be a good reason to skip).

/retest

Codecov Report

Merging #1141 (f257538) into main (e2e1680) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1141      +/-   ##
==========================================
- Coverage   62.89%   62.88%   -0.02%
==========================================
  Files         106      106
  Lines        6414     6431      +17
==========================================
+ Hits         4034     4044      +10
- Misses       1994     2000       +6
- Partials      386      387       +1

Impacted Files
Coverage Δ

pkg/matcher/repo_runinfo_matcher.go
33.33% <100.00%> (+2.56%)
:arrow_up:

pkg/provider/provider.go
75.86% <0.00%> (-24.14%)
:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

the e2e-test failure on TestGithubPush seems legit

the e2e-test failure on TestGithubPush seems legit

I tried to run test on main branch and i see its failing so not sure whether the failure is because of PR change or what
Retrying CI run one more time to make sure

Ah its passed
@chmouel could you take a look if its okay to merge then we can add this fix in 1.9.x build

authors: chmouel codecov-commenter savitaashture

repo: openshift-pipelines/pipelines-as-code


text:
Reduce noise from rhcos job

And have proper, maybe aggregate, but surely actionable info through https://issues.redhat.com/browse/ART-3953

/lgtm

authors: joepvd locriandev

repo: openshift/aos-cd-jobs


text:
CONSOLE-2892: Allow dynamic plugins to proxy to services on the cluster

Based on https://github.com/openshift/enhancements/pull/915
ccing @vojtechszocs
/assign @spadgett @christianvogt

PR is ready for final review.
QE Approver:
/assign @yapei
Docs Approver:
/assign @opayne1
PX Approver:
/assign @RickJWagner
Console Approver:
/assign @spadgett

/lgtm

/label docs-approved

/label docs-approved

@spadgett I've addressed your comments. lets the service CA with lowercased starting 's' since kubernetes docs are using it this way as well.
PTAL

verified the changes in console-operator PR and it works
/label qe-approved

authors: RickJWagner jhadvig opayne1 yapei

repo: openshift/api


text:
Add documentation for functions in iam.go

We're missing a lot of documentation and specifics for functions in the iam.go file. This PR adds descriptions and basic list of arguments for each function.

/lgtm

/lgtm

authors: clcollins fahlmant jharrington22

repo: openshift/aws-account-operator


text:
Bump dependencies to upstream 1.24

Update dependencies to versions based on upstream v1.24

/assign @thejasn @alebedev87

/label px-approved
/label qe-approved
/label docs-approved

repo: openshift/aws-load-balancer-operator


text:
Update leader election timeouts to handle api server disruptions on SNO

To be able to handle api server disruption on SNO, the leader election timeouts needs to be adjusted according to openshift/library-go/pull/1104.
Slack: https://coreos.slack.com/archives/C02NZBANL3G/p1641823800130400

/assign @alexanderConstantinescu
/assign @mfojtik

/retitle Bug 2033751: Update leader election timeouts to handle api server disruptions on SNO

/lgtm

In the library code there seems to be a distinction between HighlyAvailableTopologyMode and SingleReplicaToplogy. This is not taken into account in this PR, but I assume it might not be that important at this point and for this component?

@alexanderConstantinescu: library-go also has a function LeaderElectionDefaulting which sets the defaults accordingly. But I didn't want to import the package only cause of this function.

@abhat could you have a look at this PR? It is for https://coreos.slack.com/archives/C02NZBANL3G/p1641823800130400
/assign @abhat

@alexanderConstantinescu: library-go also has a function LeaderElectionDefaulting which sets the defaults accordingly. But I didn't want to import the package only cause of this function.

Indeed, this is ultimately going to be upstreamed (long story), so avoiding as many openshift libraries as possible would be good.

/approve

authors: creydr mfojtik squeed


text:
OCPBUGS-10809: feat: add workload annotations

added logic to merge in workload annotations to deployments

/test e2e-gcp-ovn-techpreview

/assign @JoelSpeed
/test e2e-gcp-ovn-techpreview

Verified that for the GCP deployments. The below deployments have the correct annotations added.

capi-operator-controller-manager
capi-controller-manager
capg-controller-manager

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-capi-operator/107/pull-ci-openshift-cluster-capi-operator-main-e2e-gcp-ovn-techpreview/1640361555725914112/artifacts/e2e-gcp-ovn-techpreview/gather-extra/artifacts/deployments.json

/cc @damdo

/approve
/lgtm

@eggfoobar do we need backporting for this?

@eggfoobar do we need backporting for this?

Yeah, luckily we just need to bring it back to 4.13

authors: JoelSpeed damdo eggfoobar

repo: openshift/cluster-capi-operator


text:
WIP: bump library-go to proof events fixes

openshift/library-go@a39bf282: fileObserver: detect file existence when adding reactor
openshift/library-go@b833546a: Fix serving-certs race for controller's NewCommand*
openshift/library-go@609e1511: Fix ApplySecret for type changes
openshift/library-go@114736d4: cmdbuilder: add WithVersion() that reports version metrics of binary
openshift/library-go@eca66601: cmd: add WithVersion to StartController
openshift/library-go@f0aea96b: events: wire graceful broadcaster shutdown
openshift/library-go@ada32385: events: add prometheus counter for total events

/retest

/retest

@deads2k looking at events.json in artifacts, I think this is working as we want :-)

/lgtm

authors: mfojtik sttts


text:
Add CONTRIBUTING.md

New CONTRIBUTING.md for directions on how to submit commits

/cc @ybettan

/cc @ybettan

/retest

/retest

/retest

/lgtm

authors: enriquebelarte ybettan

repo: openshift/driver-toolkit


text:
remotemachineset: periodically reconcile MachinePools for update

since we don't have watches on the remote clusters to trigger on changes
in MachineSets created for MachinePools, we need a way to periodically
sync the status for our objects.
this adds an periodic source for watch to the controller, that adds all
the MachinePools for reconcile every duration(30 minutes) with some
jitter.
xref:  https://issues.redhat.com/browse/HIVE-1511
xref: https://issues.redhat.com/browse/HIVE-1510

/assign @dgoodwin
/cc @joelddiaz

/lgtm

authors: abhinavdahiya dgoodwin

repo: openshift/hive


text:
Remove duplicate syncset status entries

Fixes existing status entries with duplicate entries

/lgtm
What can I check tomorrow morning to know we're ok?

authors: csrwng dgoodwin


text:
Fix hosted cluster being active after submanger shutdown

Why
When the hosted cluster is unresponsive from hostedcluster resources, then the corresponding sub-manger will shut down. If the same cluster is active again, then it may not create a new sub-manger since it's in the map .  This PR will delete hostedcluster from the map once shut-down the submanger.

/lgtm

repo: openshift/hypershift-logging-operator


text:
Move high-level cert secret reconciliation funcs into support/certs

Currently, there is the support/certs package that provides relatively
low-level functions for generating certificates. On top of that we have
higher level functions in
control-plane-operator/controllers/hostedcontrolplane/pki that provide
cert and ca secret reconciliation logic. This logic is not specific to
the CPO and needed in other places.
This change moves the CA and signed cert secret reconciliation logic
into support/certs, which leaves only the code for specific cert
configuration and tests in the pki package.
This change is a pure refactor that should not be changing anything and
precondition to start using the same certs/pki code everywhere.

What this PR does / why we need it:
Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #
Checklist

[ ] Subject and description added to both, commit and PR.
[ ] Relevant issues have been referenced.
[ ] This change includes docs.
[ ] This change includes unit tests.

/retest-required

Included in https://github.com/openshift/hypershift/pull/1657

repo: openshift/hypershift


text:
Enabled automatic merge on instaslice repo

Enable automatic merge using - https://docs.ci.openshift.org/docs/how-tos/onboarding-a-new-component/#enabling-automatic-merges

/assign @empovit

Fixed by openshift/release/pull/56822

authors: empovit harche

repo: openshift/instaslice-operator


text:
Adding support for multitag

Adding exec function for card:
https://trello.com/c/Yl4qomsw/878-5-additional-jenkins-plugin-enhancement-requests-from-github
ptal @gabemontero @bparees
Enabling the following forms:

1 destination stream and 1 destination tag   (i.e. single tag applies to single stream)
1 destination stream and N destination tags   (i.e. all tags apply to the same stream)
1 destination tag and N destination streams	(i.e. all streams will get the same tag)
N destination streams and N destination tags	(i.e. each index will be combined to form a unique stream:tag combination)

Oh, just noticed the default test failure after my review comment.  We'll need to look into that as well prior to merging.

Looks like the extended test set up failed @jupierce - see if this is a known flake or not in openshift/origin

@gabemontero I hand tested multi-project/multi-token configs. Looks good so far, but will need to implement extended tests to be sure those work on new 1->N and N->N variants.

@jupierce roger that - sounds good.

[test]

rebasing for conflict

@gabemontero PR test flake has been addressed.

Cool - thanks for the confirm @jupierce
On Thu, Oct 6, 2016 at 11:12 AM, Justin Pierce notifications@github.com
wrote:

@gabemontero https://github.com/gabemontero PR test flake has been
addressed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/jenkins-plugin/pull/71#issuecomment-251992831,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADbadGe7B1YVThLUN7niYvfwAdWj1fFjks5qxQ_KgaJpZM4KOHIi
.

authors: gabemontero jupierce

repo: openshift/jenkins-plugin

license_source: bigquery


text:
Change GetServiceCIDR() to GetServiceCIDRs() for dual-stack support

This has been the subject of FIXMEs for a while...
AFAIK this is used by cluster-kube-apiserver-operator and cluster-kube-controller-manager-operator, and both of their operands have been updated to be able to deal with multiple values.

/lgtm

Is the PR already filed on kube-apiserver the only downstream consumer of this function?

authors: danwinship squeed

repo: openshift/library-go


text:
OLS-1160: Bump-up starlette - fix DoS

Description
Bump-up starlette - fix DoS
Type of change

[ ] Refactor
[ ] New feature
[ ] Bug fix
[x] CVE fix
[ ] Optimization
[ ] Documentation Update
[ ] Configuration Update
[ ] Bump-up dependent library
[ ] Bump-up library or tool used for development (does not change the final image)
[ ] CI configuration change
[ ] Konflux configuration change

Related Tickets & Documents

Related Issue #OLS-1160

/approve
/hold

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.92%. Comparing base (b6cc49b) to head (ccbd9b9).
Report is 50 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1765      +/-   ##
==========================================
+ Coverage   94.90%   94.92%   +0.02%
==========================================
  Files          60       60
  Lines        2689     2701      +12
==========================================
+ Hits         2552     2564      +12
  Misses        137      137

/override "Red Hat Konflux / ols-enterprise-contract / lightspeed-service"

/unhold

authors: codecov-commenter tisnik

repo: openshift/lightspeed-service


text:
Use mongod to stop server instead of kill or pkill.

This method of stopping mongod is equivalent to using kill -2 (https://docs.mongodb.com/manual/tutorial/manage-mongodb-processes/#use-shutdown).
This change is a part of removing dependencies of scripts on mongodb options - it necessary to allow using custom config file in #147

LGTM
@bparees @mfojtik could any of you merge please?

[test]

@rhcarvalho @bparees @mfojtik Rebased. Any other suggestions?

[test]

lgtm, will merge if it passes tests.

authors: bparees mfojtik omron93 rhcarvalho

repo: openshift/mongodb


text:
CFE-935: [V2] Implement a release collector relying on local registry as storage

This PR introduces a new implementation for the Release collector - LocalStorageCollector.

LocalStorageCollector relies on an internal registry, and copies all images to that registry while mirroring to disk.

Sample command: ./bin/oc-mirror --v2 --config iscv2.yaml file:///home/myuser/release
The internal registry mirrorToDisk will use the /home/myuser/release as its local storage
Cincinatti API is used to determine the list of releases to mirror from the ImageSetConfig's Platform section
This list is saved under the working-dir/release-filters, along with the Platform section. The filename corresponds to a md5 hash of the Platform section

Upon mirroring from disk to mirror, LocalStorageCollector leverages the filter file stored under working-dir to determine the list of images to copy
images are copied from the internal registry to the destination registry
Sample command: ./bin/oc-mirror --v2 --config iscv2.yaml --from file:///home/myuser/release docker://enclave.internal/enclave

It also modifies the location of the main working directory - working-dir. Instead of it being created under the current folder (where the execution is happening), it is created under the local storage folder. This will facilitate mirroring to the enclave, as the local storage becomes the only directory that needs to be transported to the enclave for the enclave mirroring to start.

Other minor changes:

A DeepCopy method was added for type v1alpha2.Platform: The Platform structure contents are modified by v2/pkg/release/cincinatti.go during the GetReleaseReferenceImages call. It was therefore necessary to keep a copy of it before the call for calculating the release-filters filename (hash of Platform)

/hold

/unhold

@sherine-k - I'm verifying locally using a simple single release (as my internet is slow)

Verified locally using the following ImageSetConfig
piVersion: mirror.openshift.io/v1alpha2
kind: ImageSetConfiguration
storageConfig:
  local:
    path: /tmp/lmz-cfe935
mirror:
  platform:
    architectures:
      - "amd64"
    channels:
      - name: stable-4.12
        minVersion: 4.12.0
        maxVersion: 4.12.0

Executed the following command for mirrotToDisk
build/oc-mirror --v2 --config isc-copy.yaml file://cfe-935

Output from logs
build/oc-mirror --v2 --config isc-copy.yaml file://cfe-935
2023/09/25 12:43:20  [INFO]   : mode mirrorToDisk
2023/09/25 12:43:20  [INFO]   : local storage registry will log to /home/lzuccarelli/go/src/github.com/openshift/oc-mirror/v2/logs/registry.log
2023/09/25 12:43:20  [INFO]   : starting local storage on :5000
2023/09/25 12:43:20  [INFO]   : processing minimum version 4.12.0 and maximum version 4.12.0
2023/09/25 12:43:21  [INFO]   : Found update 4.12.0
2023/09/25 12:43:21  [WARN]   : signature for 4c5a7e26d707780be6466ddc9591865beb2e3baa5556432d23e8d57966a2dd18 not in cache
2023/09/25 12:43:21  [INFO]   : signature 4c5a7e26d707780be6466ddc9591865beb2e3baa5556432d23e8d57966a2dd18
2023/09/25 12:43:21  [INFO]   : content {"critical": {"image": {"docker-manifest-digest": "sha256:4c5a7e26d707780be6466ddc9591865beb2e3baa5556432d23e8d57966a2dd18"}, "type": "atomic container signature", "identity": {"docker-reference": "quay.io/openshift-release-dev/ocp-release:4.12.0-x86_64"}}, "optional": {"creator": "Red Hat OpenShift Signing Authority 0.0.1"}}
2023/09/25 12:43:21  [INFO]   : image found : quay.io/openshift-release-dev/ocp-release:4.12.0-x86_64
2023/09/25 12:43:21  [INFO]   : public Key : 567E347AD0044ADE55BA8A5F199E2F91FD431D51
2023/09/25 12:43:21  [INFO]   : copying  quay.io/openshift-release-dev/ocp-release:4.12.0-x86_64
2023/09/25 12:44:02  [INFO]   : total release images to copy 184
2023/09/25 12:44:02  [INFO]   : related images length 0
2023/09/25 12:44:02  [INFO]   : images to copy (before duplicates) 0
2023/09/25 12:44:02  [INFO]   : total operator images to copy 0
2023/09/25 12:44:02  [INFO]   : total additional images to copy 0
2023/09/25 12:44:02  [INFO]   : images to mirror 184
2023/09/25 12:44:02  [INFO]   : batch count 23
2023/09/25 12:44:02  [INFO]   : batch index 0
2023/09/25 12:44:02  [INFO]   : batch size 8
2023/09/25 12:44:02  [INFO]   : remainder size 0
2023/09/25 12:44:02  [INFO]   : starting batch 0
2023/09/25 12:50:42  [INFO]   : completed batch 0
2023/09/25 12:50:42  [INFO]   : starting batch 1
2023/09/25 12:51:43  [INFO]   : completed batch 1
2023/09/25 12:51:43  [INFO]   : starting batch 2
2023/09/25 12:54:27  [INFO]   : completed batch 2
2023/09/25 12:54:27  [INFO]   : starting batch 3
2023/09/25 12:57:58  [INFO]   : completed batch 3
2023/09/25 12:57:58  [INFO]   : starting batch 4
2023/09/25 12:58:59  [INFO]   : completed batch 4
2023/09/25 12:58:59  [INFO]   : starting batch 5
2023/09/25 13:00:03  [INFO]   : completed batch 5
2023/09/25 13:00:03  [INFO]   : starting batch 6
2023/09/25 13:01:35  [INFO]   : completed batch 6
2023/09/25 13:01:35  [INFO]   : starting batch 7
2023/09/25 13:02:22  [INFO]   : completed batch 7
2023/09/25 13:02:22  [INFO]   : starting batch 8
2023/09/25 13:03:52  [INFO]   : completed batch 8
2023/09/25 13:03:52  [INFO]   : starting batch 9
2023/09/25 13:04:45  [INFO]   : completed batch 9
2023/09/25 13:04:45  [INFO]   : starting batch 10
2023/09/25 13:08:53  [INFO]   : completed batch 10
2023/09/25 13:08:53  [INFO]   : starting batch 11
2023/09/25 13:10:27  [INFO]   : completed batch 11
2023/09/25 13:10:27  [INFO]   : starting batch 12
2023/09/25 13:12:59  [INFO]   : completed batch 12
2023/09/25 13:12:59  [INFO]   : starting batch 13
2023/09/25 13:22:00  [INFO]   : completed batch 13
2023/09/25 13:22:00  [INFO]   : starting batch 14
2023/09/25 13:24:01  [INFO]   : completed batch 14
2023/09/25 13:24:01  [INFO]   : starting batch 15
INFO[2640] PurgeUploads starting: olderThan=2023-09-18 13:27:20.118271965 +0200 CEST m=-602159.963301506, actuallyDelete=true
INFO[2640] Purge uploads finished.  Num deleted=0, num errors=0
2023/09/25 13:42:15  [INFO]   : completed batch 15
2023/09/25 13:42:15  [INFO]   : starting batch 16
2023/09/25 13:44:09  [INFO]   : completed batch 16
2023/09/25 13:44:09  [INFO]   : starting batch 17
2023/09/25 13:45:39  [INFO]   : completed batch 17
2023/09/25 13:45:39  [INFO]   : starting batch 18
2023/09/25 13:50:55  [INFO]   : completed batch 18
2023/09/25 13:50:55  [INFO]   : starting batch 19
2023/09/25 13:53:34  [INFO]   : completed batch 19
2023/09/25 13:53:34  [INFO]   : starting batch 20
2023/09/25 13:54:16  [INFO]   : completed batch 20
2023/09/25 13:54:16  [INFO]   : starting batch 21
2023/09/25 14:02:00  [INFO]   : completed batch 21
2023/09/25 14:02:00  [INFO]   : starting batch 22
2023/09/25 14:02:50  [INFO]   : completed batch 22
2023/09/25 14:02:50  [INFO]   : start time: 2023-09-25 12:43:20.117765789 +0200 CEST m=+0.036192319
collection time: 2023-09-25 12:44:02.758864935 +0200 CEST m=+42.677291457
mirror time: 2023-09-25 14:02:50.378389747 +0200 CEST m=+4770.296816279

Executed the following command for diskToMirror
build/oc-mirror -p 6000 --v2 --config isc-copy.yaml --from file://cfe-935 docker://localhost:5000/cfe-935 --dest-tls-verify=false

Output
2023/09/25 14:22:43  [INFO]   : mode diskToMirror
2023/09/25 14:22:43  [INFO]   : local storage registry will log to /home/lzuccarelli/go/src/github.com/openshift/oc-mirror/v2/logs/registry.log
2023/09/25 14:22:43  [INFO]   : starting local storage on :6000
2023/09/25 14:22:43  [INFO]   : total release images to copy 184
2023/09/25 14:22:43  [INFO]   : related images length 0
2023/09/25 14:22:43  [INFO]   : images to copy (before duplicates) 0
2023/09/25 14:22:43  [INFO]   : total operator images to copy 0
2023/09/25 14:22:43  [INFO]   : total additional images to copy 0
2023/09/25 14:22:43  [INFO]   : images to mirror 184
2023/09/25 14:22:43  [INFO]   : batch count 23
2023/09/25 14:22:43  [INFO]   : batch index 0
2023/09/25 14:22:43  [INFO]   : batch size 8
2023/09/25 14:22:43  [INFO]   : remainder size 0
2023/09/25 14:22:43  [INFO]   : starting batch 0
2023/09/25 14:22:46  [INFO]   : completed batch 0
2023/09/25 14:22:46  [INFO]   : starting batch 1
2023/09/25 14:22:46  [INFO]   : completed batch 1
2023/09/25 14:22:46  [INFO]   : starting batch 2
2023/09/25 14:22:47  [INFO]   : completed batch 2
2023/09/25 14:22:47  [INFO]   : starting batch 3
2023/09/25 14:22:50  [INFO]   : completed batch 3
2023/09/25 14:22:50  [INFO]   : starting batch 4
2023/09/25 14:22:50  [INFO]   : completed batch 4
2023/09/25 14:22:50  [INFO]   : starting batch 5
2023/09/25 14:22:51  [INFO]   : completed batch 5
2023/09/25 14:22:51  [INFO]   : starting batch 6
2023/09/25 14:22:52  [INFO]   : completed batch 6
2023/09/25 14:22:52  [INFO]   : starting batch 7
2023/09/25 14:22:53  [INFO]   : completed batch 7
2023/09/25 14:22:53  [INFO]   : starting batch 8
2023/09/25 14:22:54  [INFO]   : completed batch 8
2023/09/25 14:22:54  [INFO]   : starting batch 9
2023/09/25 14:22:54  [INFO]   : completed batch 9
2023/09/25 14:22:54  [INFO]   : starting batch 10
2023/09/25 14:22:57  [INFO]   : completed batch 10
2023/09/25 14:22:57  [INFO]   : starting batch 11
2023/09/25 14:22:58  [INFO]   : completed batch 11
2023/09/25 14:22:58  [INFO]   : starting batch 12
2023/09/25 14:22:59  [INFO]   : completed batch 12
2023/09/25 14:22:59  [INFO]   : starting batch 13
2023/09/25 14:23:06  [INFO]   : completed batch 13
2023/09/25 14:23:06  [INFO]   : starting batch 14
2023/09/25 14:23:07  [INFO]   : completed batch 14
2023/09/25 14:23:07  [INFO]   : starting batch 15
2023/09/25 14:23:13  [INFO]   : completed batch 15
2023/09/25 14:23:13  [INFO]   : starting batch 16
2023/09/25 14:23:17  [INFO]   : completed batch 16
2023/09/25 14:23:17  [INFO]   : starting batch 17
2023/09/25 14:23:17  [INFO]   : completed batch 17
2023/09/25 14:23:17  [INFO]   : starting batch 18
2023/09/25 14:23:19  [INFO]   : completed batch 18
2023/09/25 14:23:19  [INFO]   : starting batch 19
2023/09/25 14:23:21  [INFO]   : completed batch 19
2023/09/25 14:23:21  [INFO]   : starting batch 20
2023/09/25 14:23:22  [INFO]   : completed batch 20
2023/09/25 14:23:22  [INFO]   : starting batch 21
2023/09/25 14:23:26  [INFO]   : completed batch 21
2023/09/25 14:23:26  [INFO]   : starting batch 22
2023/09/25 14:23:26  [INFO]   : completed batch 22
2023/09/25 14:23:26  [INFO]   : start time: 2023-09-25 14:22:43.39984148 +0200 CEST m=+0.045520073
collection time: 2023-09-25 14:22:43.401560672 +0200 CEST m=+0.047239260
mirror time: 2023-09-25 14:23:26.894713524 +0200 CEST m=+43.540392109`

All working :)
Great work @sherine-k :100:

authors: lmzuccarelli sherine-k

repo: openshift/oc-mirror


text:
Document the downstream SDK repo

Description of the change:
Document the downstream SDK repo. Update the UPSTREAM-MERGE.sh script to put the upstream README.md into README-sdk.md.
Motivation for the change:
Document the downstream repo.
Checklist
If the pull request includes user-facing changes, extra documentation is required:

[ ] Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
[ ] Add or update relevant sections of the docs website in website/content/en/docs

/cc estroz

@camilamacedo86 and who ever else is reviewing. What you are seeing in this PR are 2 things.

@camilamacedo86 I redid the commits so that the diff is more obvious of what I'm actually changing.

repo: openshift/ocp-release-operator-sdk


text:
refactor: helper functions to generate placement structs

Added helper functions to remove code duplicate code.
Signed-off-by: RAJAT SINGH rajasing@redhat.com

@umangachapagain  above is disscussed here and here

Why is reducing duplication not inherently valuable? I believe we've already run into scenarios where we forgot to update some components when we decided to implement a change that was supposed to affect all components.
The functions also allow us to generate structs that are largely identical save for one field. If  we did it with vars/consts we'd have large structs that were largely the same. Again, why is this not valuable?

@umangachapagain do you have any reviews in it , otherwise @jarrpa  can go ahead and merge this!

Why is reducing duplication not inherently valuable?

It's never inherently valuable. Even if it is, it's negligible in this case.

I believe we've already run into scenarios where we forgot to update some components when we decided to implement a change that was supposed to affect all components.

I don't see this re-factor helping us fix those issues. Even with this re-factor one can mess it up (vs the variables which was pretty obvious).
What about when we want to make changes exclusive to a single component? We then add a bunch of if..else which will then blow out of proportion?

The functions also allow us to generate structs that are largely identical save for one field. If we did it with vars/consts we'd have large structs that were largely the same.

But, is it always going to be identical? Is there no possibility that we'd maybe like to configure placement for each component differently (other than the name)?

Again, why is this not valuable?

It's not "not valuable". It's just not significantly valuable. Here's why I think so

it doesn't significantly simplify the code
it doesn't make it easier to custom configure each component (we get what the function throws at us)
retrieving values would require less cycles then generating values
this file is still largely static. The configuration would probably change once in a release or two (functions just feels like an overkill)

I'm not against this re-factor, but I believe it's half way there.
It reduced duplication at the cost of configurability and extra processing cycle.
If it can give me configurability, I'd be more than happy with it. But, that's just me.
Since it's not a very big change, I'm not blocking this with my comment.
I'd like the value add of this refactor to be explained (PR and Commit descriptions) and justified rather than just calling it a "refactor".

@umangachapagain above is disscussed here and here

Can't reach those comments.

I considered your changes @jarrpa and I think if I start making changes that you suggested, it will make code more complicated and in turn, the whole point of "refactoring" will be of no use.
I think It's better to close it?.
@umangachapagain wdyt?.

I completely disagree. Refactoring is more than just simple code. I hate seeing the same exact framework or even exact configuration duplicated in multiple places. While Umanga has a point that these things don't change very frequently (if some would at all), that is no reason to just let the error potential sit around unaddressed. It's all just BZs waiting to come up in QE.

@jarrpa @umangachapagain
I have added more specific commit and description. Also, based on the suggestion above, I have modified the code in a way that each struct can be modified based on their own individual's value.

Code looks good, now merge the two commits. :) We don't need to know how you reacted to the review comments in the commit history, just have a clean commit that can be understood (and reverted later if needed).

Absolutely!. When I was reviewing the code myself, I forgot about to review the older commit sadly. But nvm, I'll squash them and form a new commit.

Commit fixed!.

I hate it when CI runs on local but fails on remote 👎 .

authors: jarrpa rajatsing umangachapagain

repo: openshift/ocs-operator


text:
WINC-925: Fast forward to v0.20.0

Ran commands:
git remote add upstream git@github.com:prometheus-community/windows_exporter.git
git checkout -b pull-v0.20.0 v0.20.0
git remote add openshift git@github.com:openshift/prometheus-community-windows_exporter.git
git pull openshift master --rebase
go mod tidy && go mod vendor
git push --tags

/lgtm

I have pushed the v0.20.0 tag. Steps followed on my local repo:
git fetch upstream --tags
git push origin v0.20.0

authors: aravindhp mansikulkarni96 sebsoto

license: MIT


text:
OCM-7566 | chore: bump ocm sdk to version 0.1.416

WHAT
This adds changes in the SDK to include the updated OCM cert expiry for Windows
JIRA - https://issues.redhat.com/browse/OCM-7566

/cherrypick release_1.2.38

repo: openshift/rosa


text:
The PHP_INI_SCAN_DIR can be set to multiple directories (it breaks run script)

When trying to include custom directory within a source which include the custom .ini file, users have to set the PHP_INI_SCAN_DIR=/opt/app-root/src/php:/etc/opt/rh/rh-php56/php.d
This is supported by PHP and works, problem is this line in the run script: https://github.com/openshift/s2i-php/blob/master/5.6/s2i/bin/run#L22
this will explode and opcache will not be saved... however the httpd will run ;-)

If pr #96 is merged,  It seems simpler and consistent to use default value to ":/opt/app-root/etc/php.d" and drop this line (and save 10-opcache.ini  there)

So:
export PHP_INI_SCAN_DIR=${PHP_INI_SCAN_DIR:-:/opt/app-root/etc/php.d}

envsubst < /opt/app-root/etc/php.ini.template > $PHPRC
envsubst < /opt/app-root/etc/php.d/10-opcache.ini.template > /opt/app-root/etc/php.d/10-opcache.ini

@remicollet I will make the fix to run script in #96

authors: mfojtik remicollet


text:
WRKLDS-1037: csv: fix annotation

SSIA

/override ci/prow/e2e-aws-operator

/override ci/prow/e2e-aws-operator

repo: openshift/secondary-scheduler-operator


text:
Build test-images test flake

Sometimes our test end with You do not have necessary test images, be sure to run 'hack/build-test-images.sh' beforehand. This is one example of such error: https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_sti/247/console . It would be good to nail down the reason for that.

@soltysh can be closed now?

It's still valid, the linked issue is one proof of that.

this turned out to be an issue in our jenkins infrastructure.

authors: bparees mfojtik soltysh

repo: openshift/source-to-image


text:
removed publishers for job

This PR removes:
publisher for job devtools-test-end-to-end-openshift.io-planner-api-test job

Can one of the admins verify this patch?

repo: openshiftio/openshiftio-cico-jobs


text:
Cuda out of memory

Tried to train Transformer Transducer model on Librispeech "train-clean-100" dataset on 16GB GPU. But getting "CUDA out of memory" error . Also tried by splitting various layers on 3 GPUs, 16GB each, but getting same thing. And error is pointing to ''joint'' layer, maybe because large size of tensors in joint layer.
Details:
Number of MFCCs = 128
Timesteps =512
Vocabulary size = 21800 ( tried to reduce it to 5K, but getting same thing)
Embedding layer dimension =Vocab_size*512
Audio Encoder = TransformerTransducerEncoder
Label Encoder =TransformerTransducerDecoder
loss = RNNTLoss
If anyone trained Transformer Transducer successfully and able to get results comparable with https://arxiv.org/abs/2002.02562 , please let me know the number of accelerators and their respective memory capacity.

Unfortunately, it takes a lot of memory to learn the speech recognition model.
In particular, the transducers model needs more.

Hi  @sooftware  , Thanks for your reply. Do you have idea which GPU would be good to handle this Transformer Transducer model?

Of course, the bigger the better. If several A100s are possible... That would be good.
Or it would be a good idea to reduce the mfcc coefficient and vocab size.
Transducers will use a lot of memory depending on the vocab size.

Yes, reducing the tensors dimension by various ways will work. But the thing is, I'm trying to achieve the same WER as mentioned in paper. And reducing dimensions may effect the results.

Then, I think you have no choice but to use a lot of GPUs. 😂

repo: openspeech-team/openspeech


text:
CMS SignerInfo signatureAlgorithm, what gives?

I'm a little lost on what the signatureAlgorithm is supposed to be, and it seems to vary depending on key algorithm, at least looking at our implementation.  Looking at the code we produce a Subject Public Key Algorithm for RSA (https://tools.ietf.org/html/rfc3279#section-2.3.1), while we produce a Signature Algorithm for DSA (https://tools.ietf.org/html/rfc3279#section-2.2.2) and ECDSA (https://tools.ietf.org/html/rfc3279#section-2.2.3).
Evidence for RSA:
https://github.com/openssl/openssl/blob/8158cf209792f7a92f0812ac89a9f54950e8453b/crypto/rsa/rsa_ameth.c#L812-L815
https://github.com/openssl/openssl/blob/8158cf209792f7a92f0812ac89a9f54950e8453b/crypto/rsa/rsa_ameth.c#L822
Evidence for DSA (snid is the composed digest-sign NID):
https://github.com/openssl/openssl/blob/8158cf209792f7a92f0812ac89a9f54950e8453b/crypto/dsa/dsa_ameth.c#L492-L497
Evidence for ECDSA:
https://github.com/openssl/openssl/blob/8158cf209792f7a92f0812ac89a9f54950e8453b/crypto/ec/ec_ameth.c#L493-L498
So I wonder, why the difference?  And most of all, I wonder why the digest-sign algid should be used at all, considering that the SignerInfo structure has a separate digestAlgorithm and the Message Signature Verification Process isn't interest in a digest-sign algorithm at all
Is this possibly a bug?  Otherwise, what am I missing?  Am I reading the code wrong?
If this is indeed an correct inconsistency, is this documented in a standard somewhere?

@DDvO, you might be the guru du jour, can you shed some light on this?

I also looked at the GOST implementation, and it seems to produce Subject Public Key Algorithm (https://tools.ietf.org/html/rfc4491#section-2.3).  Evidence:
https://github.com/gost-engine/engine/blob/7c30097805cba0c62555493df6dad9f0c5d1d0f3/gost_ameth.c#L258-L261

It's also a bit interesting to find diverse comments referring to implementations using signature OIDs as "broken":
https://github.com/openssl/openssl/blob/8158cf209792f7a92f0812ac89a9f54950e8453b/crypto/cms/cms_lib.c#L337-L340
I'm not sure if this is related, though...

Looking at the code we produce a Subject Public Key Algorithm for RSA

No. In all cases we are generating an AlgorithmIdentifier (X509_ALGOR). What is confusing is the OID being used in that AlgorithmIdentifier. Sometimes we use the OID for the underlying signature algorithm and somethime the combined OID of the signature algorithm and the digest algorithm.
A SignerInfo is defined here:
https://tools.ietf.org/html/rfc5652#section-5.3
  SignerInfo ::= SEQUENCE {
    version CMSVersion,
    sid SignerIdentifier,
    digestAlgorithm DigestAlgorithmIdentifier,
    signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
    signatureAlgorithm SignatureAlgorithmIdentifier,
    signature SignatureValue,
    unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }

So we have separate fields for both the digest algorithm and the signature algorithm. Note that DigestAlgorithmIdentifier and SignatureAlgorithmIdentifier above are just:
  DigestAlgorithmIdentifier ::= AlgorithmIdentifier

and
  SignatureAlgorithmIdentifier ::= AlgorithmIdentifier

The digesit algorithm identifier is described as:

The DigestAlgorithmIdentifier type identifies a message-digest
algorithm.  Examples include SHA-1, MD2, and MD5.  A message-digest
algorithm maps an octet string (the content) to another octet string
(the message digest).

But the signature algorithm identifier is described as:

The SignatureAlgorithmIdentifier type identifies a signature
algorithm, and it can also identify a message digest algorithm.
Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
SHA-256.  A signature algorithm supports signature generation and
verification operations.  The signature generation operation uses the
message digest and the signer's private key to generate a signature
value.  The signature verification operation uses the message digest
and the signer's public key to determine whether or not a signature
value is valid.  Context determines which operation is intended.

So this explicitly allows either form of OID, i.e. either just the signature OID or the composed OID of signature and digest. I cannot see where it says what to do if the value for the DigestAlgorithmIdentifier and the SignatureAlgorithmIdentifier disagree.

It's also a bit interesting to find diverse comments referring to implementations using signature OIDs as "broken":

I suspect this is referring to a case where the digestAlgorithm has a combined signature OID, e.g. so rather than the SHA1 OID, we have DSA with SHA1. This does seem broken.

No. In all cases we are generating an AlgorithmIdentifier

[ahem] Subject Public Key Algorithms in RFC 3279 are also given as an AlgorithmIdentifier, so it's not possible to say "no" that categorically.
That being said, the examples you pointed at (which is from RFC 5652, while I was reading RFC 2630) tell me that we can choose either and will still be ok.  The DSA evidence I displayed could just as well have been done like this, in other words:
    case ASN1_PKEY_CTRL_CMS_SIGN:
        if (arg1 == 0) {
            ASN1_STRING *str = NULL;
            X509_ALGOR *alg;

            CMS_SignerInfo_get0_algs(arg2, NULL, NULL, NULL, &alg);
            str = ASN1_STRING_new();
            str->length = i2d_DSAparams(dsa, &str->data);
            X509_ALGOR_set0(alg, EVP_PKEY_id(pkey), V_ASN1_SEQUENCE, str);
        }
        return 1;

If the general consensus is that we can choose the form that we find most practical (DSA-with-sha1 is more practical than DSA because the former specifies that the parameters should be undefined, ie. V_ASN1_UNDEF), then I have no further issue, my question has been answered, albeit not quite like I had anticipated.

A better place to look than the generic text in Section 10.1.2 of RFC 5652 is at the standards that specify the encodings for particular algorithms. RFC 3279 is not relevant, as that only applies to X.509 certificates and CRLs. Standards related to use of algorithms with CMS may be found at https://datatracker.ietf.org/wg/smime/documents.
In RFC 2630, the relevant text was in Section 12.2, but that has now been replaced by Section 3 of RFC 3370. Both specify that the signature algorithm should be id-dsa-with-sha1 for DSA and rsaEncryption for RSA (RFC 5754 specifies the SHA-2 algorithms). For RSA, RFC 3370 says:

CMS implementations that include the RSA (PKCS #1 v1.5) signature algorithm MUST support the rsaEncryption signature value algorithm identifier, and CMS implementations MAY support RSA (PKCS #1 v1.5) signature value algorithm identifiers that specify both the RSA (PKCS #1 v1.5) signature algorithm and the message digest algorithm.

There is no mention for DSA of using id-dsa in the signatureAlgorithms field.
Some other relevant RFCs are RFC 4056 (RSASSA-PSS), RFC 4490 (GOST) and RFC 5753 (elliptic curve).

Okie, so the message I'm getting is that "it depends"...

The DigestAlgorithmIdentifier is separate to accommodate stream processing.  This lets the signature validation code know what hash algorithm to apply to the content since the signature algorithm identifier comes after the signedAttrs, which are hashed.
The SignatureAlgorithmIdentifier has been defined by convention to be a combination of the signature algorithm and that hash algorithm, such as: sha256WithRSAEncryption and id-Ed25519 (because it is only ever used with SHA-512) and id-Ed448 (because it it only ever used with shake256).

The SignatureAlgorithmIdentifier has been defined by convention to be a combination of the signature algorithm and that hash algorithm, such as: sha256WithRSAEncryption and id-Ed25519 (because it is only ever used with SHA-512) and id-Ed448 (because it it only ever used with shake256).

And yet, with RSA it's rsaEncryption (no "with-anything") and GOST is also coded that way...

In a certificate, rsaEncryption is used for the subject public key.
Since RFC 3370 (August 2002), sha1WithRSAEncryption and md5WithRSAEncryption are used for the CMS SignatureAlgorithmIdentifier, and the RFC 4055 defines the identifiers for SHA-224, SHA-256, SHA-384, and SHA-512 with RSA.

Since RFC 3370 (August 2002), sha1WithRSAEncryption and md5WithRSAEncryption are used for the CMS SignatureAlgorithmIdentifier, and the RFC 4055 defines the identifiers for SHA-224, SHA-256, SHA-384, and SHA-512 with RSA.

Hi Russ,
I believe this is incorrect. Section 3.2 of RFC 3370 says:

The rsaEncryption algorithm identifier is used to identify RSA (PKCS #1 v1.5) signature values regardless of the message digest algorithm employed.  CMS implementations that include the RSA (PKCS #1 v1.5) signature algorithm MUST support the rsaEncryption signature value algorithm identifier, and CMS implementations MAY support RSA (PKCS #1 v1.5) signature value algorithm identifiers that specify both the RSA (PKCS #1 v1.5) signature algorithm and the message digest algorithm.

For RSA (PKCS #1 v1.5) with SHA-2, it seems that RFC 5754, Using SHA2 Algorithms with Cryptographic Message Syntax, is a better reference than RFC 4055 (which is about certificates and CRLs). Section 3.2 of RFC 5754 says:

[RFC3370], Section 3.2, specifies the conventions for RSA with SHA-1 (RSASSA-PKCS1-v1_5) public key algorithm identifiers, parameters, public keys, and signature values.  RSA with SHA2 algorithms uses the same conventions for these public key algorithm identifiers, parameters, public keys, and signature values.  RSA (RSASSA-PKCS1-v1_5) [RFC3447] MAY be used with SHA-224, SHA-256, SHA-384, or SHA-512.  The object identifiers are taken from [RFC4055].

I interpret the "uses the same conventions" part to mean that the rule that "The rsaEncryption algorithm identifier is used to identify RSA (PKCS #1 v1.5) signature values regardless of the message digest algorithm employed" still applies when using a SHA-2 hash algorithm.
For pretty much every other signature algorithm, the standards specify to use an identifier that specifies both the signature algorithm and the hash algorithm.

Yes, RFC 3370 says that rsaEncryption with no hash algorithm MUST be supported.  The sha1WithRSAEncryption and md5WithRSAEncryption were introduced as well.  The rsaEncryption was the only one in RFC 2360, which is obsoleted by RFC 3369 and RFC 3370.  RFC 4055 defines the algorithm identifiers for RSA with SHA-224, SHA-256, SHA-384, and SHA512.
Also, PKCS #1 v1.5 encodes the algorithm identifier of the hash algorithm inside the signature.  When decrypted with the public key, the DigestInfo contains the algorithm identifier of the hash algorithm that was used.

For pretty much every other signature algorithm, the standards specify to use an identifier that specifies both the signature algorithm and the hash algorithm.

Not with GOST.  I so wish it was only with RSA this happens...  but from the looks of it, this is not deterministically standardised, so there seems indeed to be a need for key type specific solutions to cover current and possibly future variations. This does explain all the CMS specific functions in EVP_PKEY_ASN1_METHOD, something I just hadn't a grip on earlier.
The main reason I asked was to figure out the best way to handle this with the new provider mechanism without having to insert CMS specific functions in there as well.  I believe I have figured out a way that's not too ugly.  PR coming up tomorrow.

For pretty much every other signature algorithm, the standards specify to use an identifier that specifies both the signature algorithm and the hash algorithm.

Not with GOST.

We have only one hash algorithm allowed to use with a particular signature algorithm. So that's why the signature algorithm is enough in such a situation.

Yeah.  The problem I was facing was trying to get something programmatically consistent without having to resort to CMS specific backend hackery (like we have in EVP_PKEY_ASN1_METHOD).  The answer is that it's not possible.

@DDvO, you might be the guru du jour, can you shed some light on this?

Sorry that so far I had missed this question by @levitte.
I'm not an expert on CMS but there was a related discussion on potentially inconsistent CMS digest/signature algorithm specifications in #9392 where I participated.
There the focus was not which form of SignatureAlgorithmIdentifier (with or without digest alg) to use for signing but which information to use when verifying a CMS signature.
My conclusion there was that in case the DigestAlgorithmIdentifier and the SignatureAlgorithmIdentifier disagree on the digest algorithm the DigestAlgorithmIdentifier takes precedence and that the OpenSSL implementation was in line with that.
In case the SignatureAlgorithmIdentifier does not refer to a digest algorithm it should be taken from the DigestAlgorithmIdentifier.
It looks like that a CMS verification implementation could also throw an error if the DigestAlgorithmIdentifier and the SignatureAlgorithmIdentifier are inconsistent, at least like @mattcaswell I did not find a requirement in the relevant RFCs how to handle that situation.

Marking as inactive, to be closed at the end of 3.4  dev, barring further input

authors: DDvO beldmit dcooper16 levitte mattcaswell nhorman russhousley

repo: openssl/openssl


text:
Non-relevant printing of 'Invalid password argument, missing ':' within the first 5 chars'

release: openssl-3.0.5
reference: (outputs truncated)
$ man openssl-passphrase-options
  pass:password
   The actual password is password.

Hello. Illustration of a non-relevant printing, while set pass-phrase is meeting related requirements.
$ cat pass-0
pass:linux
$ openssl genpkey -algorithm ED25519 -pass pass-0 -ChaCha20
Invalid password argument, missing ':' within the first 5 chars

This is simply misunderstanding. pass: is meant to be part of the command line option argument.
I.e.:
 $ openssl genpkey -algorithm ED25519 -pass pass:linux -ChaCha20 would work.
Or:
$ cat passfile
linux
$ openssl genpkey -algorithm ED25519 -pass file:passfile -ChaCha20

would work too.

We should probably add a few examples to clarify.  Sure, there's one example in the openssl-genpkey(1) manual, but...

(outputs truncated)
$ openssl help genpkey
 -pass val           Output file pass phrase source

That file in above description leads most probably as having to be interpreted as self-explicit, which, as it is here revealed, must not. As we can observe it there is no such file involved in the operation when a pass-phrase is specified as part of the -pass argument.

Would it be helpful to rethink those help lines?  Something like this, perhaps?
 -pass val           Pass phrase source for the output file

It sounds to me, that the description of the exact usage would be reached with a such sentence.

authors: Ricky-Tigg levitte t8m


text:
OpenSSL 3.0.8 Bug in EVP_get_digestbynid

Hello OpenSSL-Experts,
I think i found an issue when calling EVP_get_digestbynid with sha384 (nid= 673). I tried it with OpenSSL 3.0.8 which failed. But OSSL 3.0.2 did not fail.
As example:
int main(){

    const EVP_MD *type = EVP_get_digestbynid(673);
    if(type == NULL){
        std::cout << "Error in " << std::hex << OpenSSL_version_num() << std::endl;
        return -1;
    }
    return 0;

}

Expected Behavior:
Main returns 0.
Current Behavior:
Main returns -1 using OpenSSL 3.0.8.

That is very strange. This works perfectly for me against 3.0.8.
Where did you get your version of 3.0.8? Did you compile it yourself? What platform are you on?

Hi, yes the library is compiled by myself, i got it from GitHub regular 3.0.8 version. Platform is linux and compiled with clang14.

After quick testing, unfortunately i used the same build commands for Openssl 1.1.1. Some of the compile options may have been deprecated as i assume. I am closing this issue.

authors: mattcaswell urbanlion


text:
DTLSv1_listen() regression on 1.1.x

The rewrite of DTLSv1_listen() makes multi client DTLS server impossible on OpenSSL 1.1.x.
DTLSv1_listen() peeks into the kernel queue, but does not consume the client hello. Therefore
the common approach, creating a new socket for the client and off loading it, is doomed to failure since the hello is pending and the application logic thinks a new client approached.
This PR seems to address the regression but sadly didn't get much attention yet:
https://github.com/openssl/openssl/pull/5024
For more details see my mail:
https://mta.openssl.org/pipermail/openssl-users/2018-August/008498.html

Is there anything I can do?

All our recent focus has been on getting the 1.1.1 release out the door - which (all being well) should happen on Tuesday. I'm hoping to get some cycles to look at this after that.

kind ping
I don't expect you to fix the problem, just have a look at it and comment on the PR, please.
I'm willing to prepare a fix myself, but I need to know how you want it to be resolved.

Oops. Thanks for the reminder and your patience. I've been working on this today. I think I have a fix, but it needs more work yet.

Oh wow! If you have something to test, just throw it at me! :-)

Please try out the fix in #7375.
In the email you linked to you mentioned the dtls_udp_echo.c program. I've tried using this with the patched version. It works fine on FreeBSD. However it doesn't seem to work on Linux. I also tried the pre-rewrite version of DTLSv1_listen() in OpenSSL 1.0.2 and that didn't work on Linux either. Most of the time it fails to complete the handshake. Every now and then it all goes through smoothly.
AFAICT linux seems to have a problem with creating multiple fds bound to the same address. So if you do this:
mainfd = socket(...);
/* Set mainfd socket opts to reuse addr and port */
bind(mainfd, ...);
/* Set mainfd in the read/write BIOs */
while (DTLSv1_listen(s, caddr) <= 0);
/* now create fd for this peer */
peerfd = socket(...);
/* Set peerfd socket opts to reuse addr and port */
bind(peerfd, ...);
connect(peerfd, ...);
/* Set peerfd as the fd for the BIOs */

Then peerfd will (most of the time) never receive any data. Sometimes it does and everything goes through smoothly. If however you do this:
mainfd = socket(...);
/* Set mainfd socket opts to reuse addr and port */
bind(mainfd, ...);
/* Set mainfd in the read/write BIOs */
while (DTLSv1_listen(s, caddr) <= 0);
/* CLOSE THE MAIN FD FIRST!!! */
close(mainfd);
/* now create fd for this peer */
peerfd = socket(...);
/* Set peerfd socket opts to reuse addr and port */
bind(peerfd, ...);
connect(peerfd, ...);
/* Set peerfd as the fd for the BIOs */

Then it works everytime. Obviously though that isn't very helpful ;-)
That's what happens on my machine anyway. It looks a bit like a kernel bug. Or possibly (more likely) me be stupid somehow.

Looks like its a linux load balancing feature:
https://lwn.net/Articles/542629/
Which leaves me wondering how you are supposed to do this on Linux.

I don't understand, why do you need SO_REUSEPORT?
SO_REUSEADDR should work just fine.

Just doing SO_REUSEADDR has the same issue.

Give me a day or two such I can test it and talk to Linux netdev folks in case this is a kernel issue.

Just doing SO_REUSEADDR has the same issue.

PBCAK. I take it back (I had a toy reproducer where it didn't make a difference...but looks like there is another problem with that). Commenting out the SO_REUSEADDR code from the dtls_udp_echo.c program solves the issue. I suspect when that code was written (a long time ago) SO_REUSEADDR support was not in the linux kernel so this conditionally compiled code never got run on linux, and it worked just fine. SO_REUSEADDR now is available on Linux but doesn't work the way dtls_udp_echo.c expects it to.
Anyway problem solved.

Grr...I meant SO_REUSEPORT not SO_REUSEADDR...I edited the above comment accordingly.

This is now fixed in master and 1.1.1 (1.1.0 is only receiving security fixes now).

I hope it's ok to comment on this issue, but the advertised approach to create a connected a socket like:
bind(peerfd, ...);
connect(peerfd, ...);

is totally flawed because this operation is not atomic.
The window of opportunity between bind() and connect() makes peerfd briefly capable to receive unfiltered messages, e.g. ClientHellos from other peers. As far as I can see, what happens next is that those messages will get dropped by SSL_read() due to failed consistency checks. Will this make SSL_read() signal an unrecoverable SSL error or will it silently carry on? That's very important even with the
Yes, one may say that UDP is unreliable by definition, but artificially dropping packets when they have already reached their destination looks fishy at best.
Unfortunately, there's no way to bind a socket both to local and remote addresses atomically on a modern system (except for macOS which has connectx() now). The reliable solution to this problem is to be always ready to receive packets from other peers on connected sockets like peerfd by peeking into them using recvfrom() and comparing their source address to the one that the socket is connected to. In particular, this means that when a packet is received from an unfamiliar peer, it must be treated as a ClientHello request.
I have this approach running reliably without DTLS and now need to implement it for DTLS. The problem with DTLSv1_listen() is that it reads datagrams until EAGAIN and thus is not suitable for processing one request at a time as far as I can see. This makes it impossible to process ClientHellos on a connected socket. Am I missing something?

This makes it impossible to process ClientHellos on a connected socket.

Maybe I'm misunderstanding what you wrote - but why would you want to use DTLSv1_listen() on a connected socket? It's whole purpose is to listen on an unconnected socket.

Thanks for your quick reply! I guess the best way to illustrate the problem is to provide a code snippet.
Please check out this gist: https://gist.github.com/neoxic/0d9314ec756d37ca4303bced49b94543

Again in a nutshell, peerfd will receive ClientHellos from other peers for some short time under certain load conditions due to a race between bind() and connect().
How will OpenSSL handle this situation? What error will SSL_read() return for a ClientHello from an arbitrary peer on a peerfd? Will it be silently skipped or will it demand that the underlying connection be closed due to having beed compromised? What can be done to mitigate this problem, in particular, is it possible to process ClientHellos on peerfd?

How will OpenSSL handle this situation? What error will SSL_read() return for a ClientHello from an arbitrary peer on a peerfd?

I believe it will just be silently skipped.

What can be done to mitigate this problem, in particular, is it possible to process ClientHellos on peerfd?

Not at the moment...at least not with any kind of libssl support. I suppose you could do something application side to check for this, but it wouldn't be pretty. I'm not sure what a good libssl solution would like like. libssl is inherently connection based.
hmmmm.....I wonder if you could have some kind of multiplexing BIO the worked with an unconnected fd that had a set of associated child BIOs (one for each peer) which were effectively "connected". The job of the parent multiplexing BIO would be to forward packets to the right child BIO based on the source of the packet.

Writing a custom BIO is always an option indeed. Though it doesn't look like an easy solution for anyone who just wants to implement reliable DTLS support.
The other way around this problem would be a DTLSv1_listen() which could process only one ClientHello request at a time. On my side, I could then check the source address of each packet and schedule either SSL_read() or DTLSv1_listen(). It could have a different name though, let's say, DTLSv1_process_hello() which would be used internally in DTLSv1_listen().

Thanks for bringing up this topic @neoxic!
I wonder how other DTLS capable SSL libraries handle this case?

Writing a custom BIO is always an option indeed. Though it doesn't look like an easy solution for anyone who just wants to implement reliable DTLS support.

I was thinking more of providing such a BIO in libssl.

The other way around this problem would be a DTLSv1_listen() which could process only one ClientHello request at a time.

This could possibly be achieved with DTLSv1_listen() as it is and using a mem BIO, i.e. you create a mem bio, and then peek at the packets in your real fd. If they should be handled by DTLSv1_listen(), then read them out and push them into the mem bio. DTLSv1_listen() then only ever reads packets from the mem bio.

I assume that we'd still have a main "listen" socket. Either that should also push it in that new bio, or DTLSv1_listen() should look at both.

I wonder how other DTLS capable SSL libraries handle this case?

I can speak only for MbedTLS which handles TLS/DTLS very consistently using a similar yet much more simple concept of basic I/O (bio) over a session, connection or whatever you call it. Please not that MbedTLS is also connection oriented, but what differs is that MbedTLS gives full control to the user over how a (D)TLS session is handled by allowing to process a handshake and a shutdown on a session in progressions, not just until the user's intervention is necessary (WANT_READ/WANT_WRITE). Necessary stages of mbedtls_handshake() return an intermediate result, thus a (D)TLS handshake can be performed in a successive manner. The same could be easily achieved in OpenSSL, for instance, if DTLSv1_listen() returned intermediate results instead of reading/writing until EAGAIN.
So, instead of handling certain stages of a handshake on the BIO level by messing around with custom multiplexing, peeking into packets, etc., it is much easier to just let the calling party know exactly what needs to be scheduled/done next by returning corresponding results from SSL_do_handshake() for example.
MbedTLS also has "demo" mbedtls_net_* functions which serve illustrative purpose and shouldn't be used in production. For DTLS, those functions have the same flaw (even worse) when a new DTLS connection is created. The new connection is created from a "main" file descriptor by first peeking into a packet with recvfrom() and then calling connect() on it effectively converting it into a session. A new "main" file descriptor is then created by calling bind() to an address fetched via getsockaddr() on the old one. This leads to a much wider window of opportunity where packets not only get lost when no socket is bound, i.e. between connect() and bind(), but they also get lost for the same reason as described above after connect() was called on a session socket (queued packets from other peers). But again, the above doesn't cause any problems because the demo functions serve educative purpose.

I assume that we'd still have a main "listen" socket. Either that should also push it in that new bio, or DTLSv1_listen() should look at both.

Needs a bit of thought to try and figure out exactly  how that would work.

The other way around this problem would be a DTLSv1_listen() which could process only one ClientHello request at a time.

This could possibly be achieved with DTLSv1_listen() as it is and using a mem BIO, i.e. you create a mem bio, and then peek at the packets in your real fd. If they should be handled by DTLSv1_listen(), then read them out and push them into the mem bio. DTLSv1_listen() then only ever reads packets from the mem bio.

I'll stick to this approach for now. Thanks, Matt!

Unfortunately, the mem BIO trick doesn't work with DTLSv1_listen() because a mem bio doesn't know what BIO_dgram_set_peer() is. Consequently, I can't generate a cookie in a cookie callback because I can't get a client's address.
Also, I was quite surprised to discover that there's no way to pass an "opaque user pointer" to a callback in OpenSSL to circumvent the above issue. The s_server.c example uses a static variable to pass a peer's address for SSL_stateless() callbacks, wow!
There seems to be no other way to implement my own custom BIO to solve this issue. OpenSSL's API looks more and more outlandish and unfriendly with each passing day I spend trying to get DTLS working reliably.

Also, I was quite surprised to discover that there's no way to pass an "opaque user pointer" to a callback in OpenSSL to circumvent the above issue.

Many callbacks do allow that - but apparently not the cookie gen callback. However it does pass a pointer to the SSL. You could use SSL_set_app_data() and SSL_get_app_data() to associate and retrieve arbitrary data with the SSL object - which you can then get at from your callback.

Oh, I didn't know about SSL_set_app_data() and SSL_get_app_data(), thanks! They could still make the mem bio trick work.

Improving the DTLSv1_listen() situation would be rather a feature as it would require API changes and additions.

Just wanted to drop in and thank @mattcaswell on the memory BIO hint as that solved accepting multiple simultaneous connections for me. The full solution maybe worth a writeup.

Marking as inactive, to be closed at the end of 3.4 dev barring further input

authors: kroeckx mattcaswell neoxic nhorman richardweinberger t8m tlsalmin


text:
dh: document what the PEM files in apps actually contain.

They were claimed to be the SKIP primes but they are really two of the MODP Diffie-Hellman groups for IKE.

[x] documentation is added or updated

Should this go back to 1.1.1 too?  The PEM files were last modified four years ago.
Should the dh1024.pem file be deleted and the reference in the documentation be removed?
I'm happy for both to happen.

The dh*.pem files in crypto were also very old and unusued.  I've removed them as well.
Probably not for 1.1.1 given the file destruction.

This pull request is ready to merge

Merged to master.  Thanks for the review.

authors: openssl-machine paulidale