Changes to utilize default +UseContainerSupport in new releases of Java

Updates in run-java.sh to use latest VM provisions and switches for running Java in containers.
-XX:InitialRAMPercentage
-XX:MaxRAMPercentage
-XX:MinRAMPercentage

The vm_test are failing owing to assertions for +UseParallelGC which is not required with the new switches and the tests are also not recognising the new switches.
Could someone please take a look and let me know what should be changed regarding testing the old assertions?

Took some time to figure out the logic of tests (artefacts).
The tests are now in sync with the new VM provisions and switches.
Could someone please check and merge?

@rhuss,
Could you please review for merge?

@rhuss,
It has been a long time.
Could you please take a look at this?

I'm travelling this week, and as mentioned, I'm super busy with other tasks and somewhat disconnected from this project. @astefanutti any ideas what the support status of this project is ?
Apologise for that, if no one will pick up I will have another look next week. But what I can already say, we won't drop the support for older Java 8 versions or maybe even not for Java 7. So introducing this support of newer flags should not ditch the other support.

It is good to see things progress rather than stalled for weeks.
I understand you are very busy but do take a look soon.
As there are no tests for Java 7 in the CI, I assumed the plan was to phase it out like the JBoss images, may be by including a notice for deprecation.
But if you don't want to use the native features of Java for Containers, it is your call.
I would just feel bad to see the efforts going to waste, after months.

I'm mostly AFK this week. I'll review it ASAP when I'm back.
I agree with @rhuss that we may not be ready to drop support for older JDK 8 versions. But the PR totally makes sense for newer JDK versions.
If we agree, I'd suggest you only update logic for newest JDK versions, either by adding runtime  switches, or by creating a JDK 8 branch and have it sourced with Fish Pepper for Java 8 images, if that is possible.

We can make use of the version (tags) for run-java-sh.
For newer versions like JDK/JRE 8 (u212), 11 and above use the new version of run-java-sh in fish-pepper (eg. 1.4.0).
For older JDK like JDK/JRE 7 and lower versions of JDK/JRE 8 ( < u212) use the relevant run-java-sh (1.3.4).

@imranzunzani can't the script itself be smart enough to detect when it it on java 7/ old java 8 ? Or at least offer an options like --legacy-default-limits or so ?
I'm not a big fan of branching off to maintain two branches.

@rhuss
I don't see the point in having logic that will become 'dead code' in the coming year.
I am talking about 'Tag' not 'Branch'.
Tags (versions) are good for such situations.

How would you add bug fixes which would also affect the 'old' version when its only a tag ?
The question is whether we still support 7 and lower  8 versions and are willing to add fixes or we say they are unsupported (but you can use the last version even with bugs). My point is that I still think that we should still support older versions (bug fixes, but no new features).
However, this should be decided by the current maintainers of this repo.

I agree with @rhuss that tagging will inevitably lead to branching and I think we still need to maintain lower Java 8 versions as well.
Having a quick look at the s2i images, I wonder whether it's actually possible to source different versions of run-java-sh per image:
https://github.com/fabric8io-images/s2i/blob/51d0e1c4e93bd414ccd90c430f84f4e5823e4bde/java/fish-pepper.yml#L17
I'm no expert in Fish Pepper, so happy to be proven wrong. Otherwise, that may help making a decision :).

I don't think it's possible on this level as fish-pepper assumes unique block names (but they would be the same on two different tags).
What I could imagine is to evaluate an environment variable in the script run-java.sh, let's say "RUNJAVA_LEGACY_LIMIT_SUPPORT" and, if set, switches back to the old behaviour. That way, images based on 7/8-old could set this env and switch on the old behaviour.
wdyt ?

I was thinking run-java.sh could run java -version and adapt accordingly. That being said, that may be too much spawning a JVM just for the version and would require some extra logic to parse the returned string. So the environment variable approach seems simpler. Either way sounds good to me.

Yes, probing for Java for every run seems to be overkill for me, too. Especially as the containing image should already know which Java version it uses.

Especially as the containing image should already know which Java version it uses.

Yes exactly. +1 for the env variable approach then.

@astefanutti,
Why do you need to maintain the older Java 8 versions when the new updates introduce fixes and new feature still maintaining the Java 8 compatibility?
@rhuss,
I am sorry but I am not a fan of coding a functionality that will become unusable soon.
It is great that you guys have the agreement on the idea but I will have to now bow out of this PR.
If you want to incorporate the changes for the new switches, you are welcome to use the work that I have done so far, as I neither am in agreement with the approach nor would I have the required time to keep changing the logic again and again, based on your ideas.
Thank you.

@imranzunzani as the run-java-sh script is used widely, it is always difficult to know what all the constraints our users can possible have w.r.t. the JDK version they are using. So I would have considered the approach of adding an environment variable a small price to pay (at our expense, as you've rightly pointed out), compared to forcing all our users to upgrade their runtimes to keep current with the run-java-sh script.

I am sorry but I am not a fan of coding a functionality that will become unusable soon.

People are using this script for various platforms and are often not able to upgrade the JVM for various reasons. A good example from the past was the very slow adaption from Java 1.4 to Java 1.5 to Java 6 which took several years. You cannot safely assume that the users of this script will all run with the required JVM machines. Of course, it's up to the maintainers to decide how far we want to go back. At this point in time, we just believe that skipping backwards compatibility right now is too painful for existing users of those images.

It is great that you guys have the agreement on the idea but I will have to now bow out of this PR.

Ok, its your choice. Thanks for the work you've done, it will be for sure a good basis for a future improvement when going t.

If you want to incorporate the changes for the new switches, you are welcome to use the work that I have done so far, as I neither am in agreement with the approach nor would I have the required time to keep changing the logic again and again, based on your ideas.

Not sure whether we really requested so many changes (at least I'm not aware of). And sorry again for the long delay for picking up the PR, it's just really that our resources were quite limited (and turnaround times in open source projects that large are no unusual, although for sure this should not be the case ;-(. Apologies for that.
thanks again...

A good example from the past was the very slow adaption from Java 1.4 to Java 1.5 to Java 6 which took several years.

The essence of using containers is to be able to rollout faster changes and updates. You are just assuming that the user base is slow to move but you may be underestimating them.
With the shorter cycles of updates of Java and LTS moving on to 11, it is sensible to get in pace with it.

Not sure whether we really requested so many changes (at least I'm not aware of).

It has been two months since we started on this matter. You mentioned about being happy to take up JDK changes in a PR and since I have worked on it, I thought it would be good to submit it to be used by the project at large.
Then I kept waiting for a response and after a month you came back and told me that the PR should be in the run-java repo which you will then use for the Java images.
I created the PR here, and again a wait for a month began for it to get reviewed with more changes in the name of keeping things compatible with the past.
I understand that resources could be limited, but may be you guys could try to be more responsive to people who are willing to contribute.
In my line of work, I have seen bad effects of having unusable sections of code which starts becoming a big headache to maintain and clean.
So, I will not do that myself.
This will look like a different mindset than the people around here but I choose to keep it.

thanks again...

Happy that I could at least nudge things towards new provisions. I have already observed containers behaving better in performance with the new provisions in place (of course it depends on the app inside too). I hope you guys would take this forward fast so that all Java containers and S2I could benefit from it.
Thank you.

The essence of using containers is to be able to rollout faster changes and updates. You are just assuming that the user base is slow to move but you may be underestimating them.
With the shorter cycles of updates of Java and LTS moving on to 11, it is sensible to get in pace with it.

It's not about rollout or updates. Some people want still Java 7 base images for legacy apps. It's about what's in the containers.

It's not about rollout or updates. Some people want still Java 7 base images for legacy apps. It's about what's in the containers.

Java 7 has gone 'end of life'. With the GA (public) updates already gone, there is only the higher versions that projects/developers could now upgrade to.
Anyway, if you are serious about supporting Java 7, do include the tests for Java 7 which are not there for run-java-sh. 8 and 11 are only covered at the time of this PR.

Closing for now.

It is disheartening to see that side (open source) projects have to endure such delays.
I have provided the clarifications for the cited minor issues.
We have moved ahead on these with custom Alpine bases for Java, in the meanwhile.
@rhuss I tried to help, and hope you will incorporate these new features soon too. A 'Happy New Year' to you too!


Support retry backoff in Zeebe 1.3

Zeebe 1.3 will support backoff retry for failed tasks. See here: https://github.com/camunda-cloud/zeebe/issues/5629.
The complete.fail() method of the job worker callback will probably need extending with an optional backoff time parameter.

Here are the gRPC API changes (not merged yet in Dec 2021): https://github.com/camunda-cloud/zeebe/pull/8359/files

From version 8.0.0 of the package, used with a 8.0.0 Zeebe broker, you can specify to the broker an optional backoff for the reactivation of the job, like this:
job.fail({errorMessage: 'Triggering a retry with a two second back-off', retryBackOff: 2000, retries: 1})


math: convert Exp to the new Context-style operations

e.g., it should be ctx.Mul( ... ) not z.Mul( ... )

was done a while ago.


Data Categorization Link on About Page has Changed

The data categorization policy AD71 has been retired and now references AD95. The link for it under the content policy needs to be changed.
https://policy.psu.edu/policies/ad95
@olendorf

Need to edit the content as an administrator to change the link text and location

Updated the page to have the correct link


Clean Up and Update Single Exchange Dialogs

The Single Exchange Dialogs section is ambiguous, too broad and out of date. Here is how you can help us improve this:

Removing links which you think do not fit in the section. Don't worry about damaging this repository, we can have a discussion on the PR you raise.
Consider adding 2-3 code examples and datasets
Consider adding 2-3 examples from slot filling (sequence mining for text is welcome too) and other approaches in chatbots

Hey @the-ethan-hunt, do consider continue contributing to awesome-nlp. Take a look at this issue if you'd like :)

Sure @NirantK ! But is there any other issue I might possibly work on? :sweat_smile:

Sure @the-ethan-hunt.
Thanks for adding Korean from #98 but did not make enough progress on Chinese, Japanese or any European languages for that matter. It'd be awesome if we'd take that issue to its due conclusion.
It saves a lot of time for the community to have all of the best tools for a particular language in one place.

Hey @anu0012, are you still interested in working on this? We could really appreciate a hand here :)

Sure @NirantK. In the second point which you mentioned what type of code examples and dataset can be added?

@anu0012 Chatbots, virtual assistants and any other popular form of conversational interfaces is a good starting point.
E.g. there is some work on chatbots from Microsoft and Facebook both, check for what datasets they've used and if we can mention them here. Similarly, there is some work on intent detection etc, maybe look if that is relevant?
If at the end of all of this search, we are still unsatisifed with the quality and breadth of coverage, maybe we can merge this section with Conversational Q&A which has similar technical challenges imho. I'd be mostly going by your (and community's) recommendation and findings on the same.


Teams Compliance Recording Application parameters not affecting

We have created a Call Recording Bot and used it to evaluate Teams Compliance Call Recording Policy.
From the parameters of New-CsTeamsComplianceRecordingApplication I noticed that:
-RequiredDuringMeeting --> which cause, if this is set to True, the user will be ejected from the meeting if the policy-based recording application leaves the meeting or is dropped from the meeting.
-RequiredDuringCall --> which cause, If this is set to True, the call will be canceled if the policy-based recording application leaves the call or is dropped from the call.
So I initiated a Call / Meeting and during the session I power off the Bot bot but nothing happened.
Is it something missed in my configuration or in Bot Code?

@fadynag , Did the bot drop from the call once you power off ?

I didn't notice anything like that. How can I check?

As per the doc :

If this is set to True, the call will be canceled if the policy-based recording application leaves the call or is dropped from the call.

So can you make sure that the bot drops/leaves the call when you are testing this.

Actually what happened is call (users) continue in the call while we Power-off the Bot. so it seems nothing happened given that Parameters are set to True.

@fadynag , The only entity that can detect whether a bot is alive or dead is the media processor, because it detects a lack of media packets from the bot and raises a media timeout after some time. The media timeout event then causes the bot's leg to get torn down, which then initiates the ejection of the user from the call / meeting.

@fadynag, Please let us know if you still have any query. We will reopen the ticket


UItableview scrolling not smooth

Hi,
You done a grade work but i m facing small issue when i increase records with around 25 tags Tableview scrolling not smooth and  some jerky.
Thanks

Hi @SalmanAhmedDev, do you mean 25 tags in one cell, or 25 tags total in one table?

Yes boss

25 tag in one cell

here is small video : https://files.fm/u/gr7qstet

I see. I think the issue comes from a bad estimation of the cell height. If you set 376.5 as estimated height, you will see the improvement. Or if all cells have the same count of tags and more than 25 in total, the issue will be gone.
I suggest you can take a look of this post: Jerky Scrolling After Updating UITableViewCell in place with UITableViewAutomaticDimension
Cheers,

Ok will update you. Thanks for your time

i have run simulator its working fin but when i run device OS 10.0.2, Still scrolling  issues :( here is video  link : https://streamable.com/d8r7r

@weijentu have you check video?

Yeah, my suggestion is to increase estimate height if you have many cells with larger height.

Yup i have increased to 400, but still not solved :(

Hmm, it's probably owing to auto layout is a mechanism for dynamic size change at runtime; therefore, some sacrifice for performance might be unavoidable.


Using data-toggle on a  with an  prevents default functionality

Using data-toggle on a <label> with a <input type="radio|checkbox"> prevents default functionality of the input toggling between the items.
OS: MacOS Sierra
Browser: Safari
JSBin:
https://jsbin.com/selasukami/edit?html,css,output
v4 alpha-6 Bootstrap

Confirmed that this is still ongoing in 4.0.0-beta. I do recall a similar issue with a lengthy discussion but cannot find it now. It would be good to know if this is expected behaviour I have to work around, or a bug that might get fixed.
Loving bootstrap! :heart:

That's due to : https://github.com/twbs/bootstrap/blob/v4-dev/js/src/tab.js#L265

Do we close this as a won't fix for v4? Still an issue in v4.5.x: https://jsbin.com/coguvof/edit?html,output.
Unclear if we need to do anything for v5 here either...

Do we close this as a won't fix for v4? Still an issue in v4.5.x: https://jsbin.com/coguvof/edit?html,output.
Unclear if we need to do anything for v5 here either...

Must be present on v5 too:
https://github.com/twbs/bootstrap/blob/7e6d7b8f817ad1212238f15124335d37c93ebfea/js/src/tab.js#L205
Patches welcome to allow this case.

Must be present on v5 too:
https://github.com/twbs/bootstrap/blob/7e6d7b8f817ad1212238f15124335d37c93ebfea/js/src/tab.js#L205
Patches welcome to allow this case.

I think that in 5-beta3 is ok, (yes  I admit I cannot understand it ).
In any case I made one more Pr

I think that in 5-beta3 is ok, (yes I admit I cannot understand it ).
In any case I made one more Pr
Example of beta-3: https://jsbin.com/hekipumuvi/edit?html,output

I think you have to use data-bs-target but it's not caused by the event.preventDefault(). There is simply no implementation for this exact case. If you step through the debugger you can see the radio is selected but then changes back.

@GeoSot Do you want to tackle this as a change in v5, or do we leave it as-is?

This sort of pattern https://jsbin.com/selasukami/edit?html,css,output seems to try and do two things at the same time. The controls should either be tab controls, or radio buttons...not both. As such, I'd suggest this is not a pattern we'd want to fix/support nor encourage in our code.


Local LLM install and use

I'm a noob to AI. how do we install SuperAGI in anaconda env and use a free local llm instead of an api key?

Hello, I am just a fellow user of this project. Most AI projects I am following actually go with an anaconda env as you indicated, some offer docker additionally. The developers of this project have opted to go the docker route.
Just as you are, I am mainly interested in using a free local llm with an agent. There is an initial integration of a dockerized Oobabooga installation that has recently been merged, but it needs further work for the interoperability:
https://github.com/TransformerOptimus/SuperAGI/issues/289#issuecomment-1590575100_
While I love Oobabooga and I can see that using a dockerized version simplifies things, I would prefer having a separate installation and just use the API Oobabooga offers and point to that at SuperAGI.

I would also prefer anaconda vs a full docker image

hopefully someone listens to our plea guys

It finally happened? Awesome! How do we apply it?

@gitihobo Sorry for the confusion, closed the issue by mistake. It is part of our next release and will be announced soon to the public

local llms is already released you can follow the steps explained in this video run your models with superagi

any possibility of adding connecting to api running on my machine without using a docker? Would like to use my existing textgenwebui or koboldAI projects


NotImplementedError: Force plate data not supported.

Hello ,
I am trying to use the optitrack that is connected to a windows machine through motive 1.7.5 and stream data to a linux machine. I tried running this package after making sure all the settings are correct. However it gives out an NotImplementedError : Force plate data not supported. Any help would be greatly appreciated. I am using ROS Indigo and Ubuntu 14.04

The error you mention is given by the python-optirx module (See here).
Could you please give more details about your setup? (Cameras, markers, rigid bodies, etc...)

4 Cameras (Optitrack Prime 17W) , 3 Markers, 1 Rigid Body

Did you manage to solve this issue?


Stablize the sorting of meta.ShardGroupInfos

Required for all non-trivial PRs

[x] Rebased/mergable
[ ] Tests pass
[ ] CHANGELOG.md updated

I definitely used the wrong wording... Stable sort would imply that elements could be equal. This change would ensure that any two ShardGroupInfo objects would not be "equal" unless they had all the same timestamps, which should not happen.


[synthetics-minion] Update synthetics-minion helm chart v3.1.6

Is this a new chart

No

What this PR does / why we need it:

Updates the synthetics-minion helm chart to use the latest containerized private minion release version v3.1.6

Which issue this PR fixes

Removes warning messages related to the Blackbird module

Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

[x] Chart Version bumped
[x] Variables are documented in the README.md
[x] Title of the PR starts with chart name (e.g. [mychartname])

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


Fix for #3312 sigpanic in TargetManager

fix for #3312
Run stop only when the TargetManager is confirmed to be started.
cc: @brian-brazil

Thanks!


test_internet: dubious regexp

tests/test_data/test_internet.py includes the following regexp fragment:
<EMAIL_ADDRESS>
But $-_ is a character range that includes digits, uppercase letter and a bunch of punctuation characters.
You probably wanted this instead:
<EMAIL_ADDRESS>
The dubious regexp was found using pydiatra.

@jwilk Can you send us PR, please?


Doesn't Play well with others

It seems to rewrite the sourcemap with absolute paths, rather than just updating the relative paths.
And i believe this is interfering with gulp-clean-css
gulp.task('sass', function () {
  var styles = gulp.src('app/assets/stylesheets/app.scss')
    .pipe(sourcemaps.init())
    .pipe(sass({
      includePaths: 'node_modules',
    }))
    .pipe(resolve({
      attempts: 1,
      root: 'public/css'
    }))
    // .pipe(useref({
    //   base: 'public/css',
    //   pathTransform: function(newAssetFile, cssFilePathRel, urlMatch, options) {
    //     var path = urlMatch
    //       .replace(/^(\.\.[\/\\])+(\.\.[\/\\])/g, '$2')
    //       .replace(/^(\.\.[\/\\])node_modules/g, '$1vendor');

    //     return path;
    //   }
    // }))
    .pipe(cleanCSS({
      level: util.env.production ? 1 : 0,
      rebaseTo: 'public',
      sourceMap: false,
    }))
    .on('error', handleError);

  if(!util.env.production)
  {
    styles = styles.pipe(sourcemaps.write());
  }

  return styles.pipe(gulp.dest('public/css'));
});

On run i get this
C:\path\node_modules\vinyl-sourcemaps-apply\node_modules\source-map\lib\source-map-generator.js:177
        throw new Error(
        ^

Error: SourceMapGenerator.prototype.applySourceMap requires either an explicit source file, or the source map's "file" property. Both were omitted.
    at SourceMapGenerator_applySourceMap [as applySourceMap] (C:\path\node_modules\vinyl-sourcemaps-apply\node_modules\source-map\lib\source-map-generator.js:177:15)
    at applySourceMap (C:\path\node_modules\vinyl-sourcemaps-apply\index.js:27:15)
    at C:\path\node_modules\gulp-clean-css\index.js:65:9
    at C:\path\node_modules\clean-css\lib\clean.js:109:9
    at C:\path\node_modules\clean-css\lib\reader\read-sources.js:26:64
    at loadOriginalSources (C:\path\node_modules\clean-css\lib\reader\load-original-sources.js:26:5)
    at Object.callback (C:\path\node_modules\clean-css\lib\reader\read-sources.js:26:14)
    at doApplySourceMaps (C:\path\node_modules\clean-css\lib\reader\apply-source-maps.js:61:23)
    at applySourceMaps (C:\path\node_modules\clean-css\lib\reader\apply-source-maps.js:33:5)
    at Object.callback (C:\path\node_modules\clean-css\lib\reader\read-sources.js:25:12)

But if i comment out resolve it seems to work fine

Try to explicitly provide the base path as noted in readme when calling the gulp.src. Did it help? Can you also provide a test case to the test/ folder?

I'm having the same problem with<EMAIL_ADDRESS>and<EMAIL_ADDRESS>When I have at least one of those in the same gulp task before gulp-resolve-url I get the same error as @dgoring.
Putting gulp-resolve-url after gulp-clean-css and/or gulp-autoprefixer doesn't generate any errors but the work done by gulp-clean-css and/or gulp-autoprefixer is not saved

Still an issue for me, any task after gulp-resolve-url that use the sourcemap fail. Looks like other packages issues, see https://github.com/terinjokes/gulp-uglify/issues/53 and https://github.com/sindresorhus/gulp-autoprefixer/issues/2


Installing just etcd fails.

Flow that is required to install just etcd requires to have at least 1 kubernetes master  (groups['kube-master'][0] is used in many places).
To start provisioning I'm using:
kargo deploy -u vagrant --ansible-opts "--tags etcd,docker,preinstall"

Good catch. We also want this fixed in order to test a separate etcd cluster for network plugins https://github.com/kubernetes-incubator/kargo/pull/660

working an separate etcd in #3616


Add rerun in Grinder link in release summary report

This is a cherry-picked PR of #638. Updated TRSS Release Summary Report to include rerun in grinder link for the entire build report. This is partial fix of #624
Fixes: #631
Signed-off-by: Tommy Tho<EMAIL_ADDRESS>

Report Preview
Release Summary Report for Test_openjdk11_hs_extended.openjdk_x86-64_linux
Report generated at: Wed, 30 Mar 2022 20:35:40 GMT
TRSS Build and TRSS Grid View
Jenkins Build URL https://ci.adoptopenjdk.net/job/Test_openjdk11_hs_extended.openjdk_x86-64_linux/30/
Started by upstream project "build-scripts/jobs/jdk11u/jdk11u-linux-x64-hotspot" build number 991 at 2021-06-26, 11:51:07 a.m.
Rerun all

Test_openjdk11_hs_extended.openjdk_x86-64_linux ⚠ UNSTABLE ⚠
java -version output
openjdk version "11.0.12" 2021-07-20

OpenJDK Runtime Environment Temurin-11.0.12+5 (build 11.0.12+5)

OpenJDK 64-Bit Server VM Temurin-11.0.12+5 (build 11.0.12+5, mixed mode)

jdk_security_infra_1 => deep history 0/1 passed | possible issues
jdk_tools_1 => deep history 0/2 passed | possible issues
Test_openjdk11_hs_extended.openjdk_x86-64_linux_testList_2 ⚠ UNSTABLE ⚠
jdk_management_1 => deep history 0/4 passed | possible issues

Report Preview
Release Summary Report for Test_openjdk11_hs_extended.openjdk_x86-64_linux
Report generated at: Wed, 30 Mar 2022 21:19:48 GMT
TRSS Build and TRSS Grid View
Jenkins Build URL https://ci.adoptopenjdk.net/job/Test_openjdk11_hs_extended.openjdk_x86-64_linux/30/
Started by upstream project "build-scripts/jobs/jdk11u/jdk11u-linux-x64-hotspot" build number 991 at 2021-06-26, 11:51:07 a.m.

Test_openjdk11_hs_extended.openjdk_x86-64_linux ⚠ UNSTABLE ⚠
Rerun all
java -version output
openjdk version "11.0.12" 2021-07-20

OpenJDK Runtime Environment Temurin-11.0.12+5 (build 11.0.12+5)

OpenJDK 64-Bit Server VM Temurin-11.0.12+5 (build 11.0.12+5, mixed mode)

jdk_security_infra_1 => deep history 0/1 passed | possible issues
jdk_tools_1 => deep history 0/2 passed | possible issues
Test_openjdk11_hs_extended.openjdk_x86-64_linux_testList_2 ⚠ UNSTABLE ⚠
Rerun all
jdk_management_1 => deep history 0/4 passed | possible issues


[stable/grafana] when persistence enabled pod is not running

I want to persist grafana. But when I enable persistence, can not start pod. I was tried both dynamic provisioner (nfs-dynamic-provisioner) and manually created persistence volume. But result is same
helm install grafana \ --namespace test \ --set persistence.enabled=true \ --set persistence.size=1Gi \ --set persistence.storageClassName=default \ stable/grafana
After this command pvc and pv created and bounded. But I think application can not store their data this storage. And I am getting:
Error from server (BadRequest): container "grafana" in pod "grafana-85f56cf56f-545hd" is waiting to start: PodInitializing
OS: Centos 7
Helm: v3.2.1
K8s: v1.18.2

📢 This chart is deprecated: https://github.com/helm/charts/pull/23662
If still applicable, please re-open issues at: https://github.com/grafana/helm-charts


[WIP] fix(module:icon): fix broken icons

PR Checklist
Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/CONTRIBUTING.md#commit
[ ] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been added / updated (for bug fixes / features)

PR Type
What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
[ ] Yes
[x] No

Other information

Fix some broken icons in components and demos.
Fix old API compatibility

@HsuanXyz Please review again.


SolrCore Initialization Failures

I am following the steps in the README.

ddev get ddev/ddev-solr
ddev restart
"Once up and running, access Solr's UI within your browser by opening http://.ddev.site:8983"

So, I go to http://.ddev.site:8983 and log in and see the following:
SolrCore Initialization Failures

dev:  org.apache.solr.common.SolrException:org.apache.solr.common.SolrException: Please check your logs for more information

So I check my logs:
Log4j2

Time (Local)
Level
Core
Logger
Message

4/22/2024, 10:18:14 PM
WARN false

ZkController
Contents  of zookeeper /security.json are world-readable; consider setting up ACLs  as described in  https://solr.apache.org/guide/zookeeper-access-control.html

4/22/2024, 10:18:14 PM
WARN false

CoreContainer
Solr  authentication is enabled, but SSL is off.  Consider enabling  SSL to protect user credentials and data with encryption.

4/22/2024, 10:18:14 PM
ERROR false
x:dev
ZkController

4/22/2024, 10:18:14 PM
ERROR false
CoreContainer
Error waiting for SolrCore to be loaded on startup

Consequences

I also note that no cores or collections have been initialized on startup (according to the solr UI).  I was expecting that one would be created in the docker compose build steps.

Sure enough, just as I feared, when I configure my connection to solr in Drupal with:

Server Name: solr
Backend: Solr
Solr Connector: Solr Cloud with Basic Auth
Solr node: solr
Solr port: 8983
Solr Path: /
Default solr collection: mycollection
http basic auth: (username and password as specified).
Save
I get the following message:

Warning message
Server solr is reachable but the configured collection is not available.

I check the solr admin page and I see that solr, as I feared, does not see my collection. I forge ahead...

On the server's "view" page click the Upload Configset button and check the "Upload (and overwrite)

Ok. I follow these instructions. Number of shards 1, etc, And I click the upload button.
I get the following error:

Error message
The configs of the essential Solr field types are missing or broken for server "solr".

So it looks like the container is failing to be initialized with something essential pre-requisite setups.
When I ssh in to the container, I see indeed that no collection has been created on the file system:
$ ls -al /var/solr/data
total 24
drwxrwx--- 4 solr solr 4096 Apr 23 02:52 .
drwxrwx--- 4 solr root 4096 Apr 23 02:12 ..
drwxr-xr-x 2 solr solr 4096 Apr 23 02:18 filestore
-rw-r--r-- 1 solr solr 2427 Jan 12 17:42 solr.xml
drwxr-xr-x 2 solr solr 4096 Apr 23 02:18 userfiles
-rw-r--r-- 1 solr solr 1048 Jan 12 17:42 zoo.cfg

Viewing the solr server I set up, I see that I can connect, but the collection is not available. "The Solr collection could not be accessed. Further data is therefore unavailable."

You can see also in the search_api_solr module output that the module is making a very odd request to solr:8983/solr/benz/admin/luke??? I have no idea why it would be requesting such a path.
I have made no changes to anything other than the ones specified in the README. No customizations to the image, or to .ddev/docker-compose.solr.yaml
Changing connection from auto detect to 8.x doesn't help.
Trying to make it work
Using the solr admin UI, I created a collection, using the same name as the failed attempt from the search_api_solr module.
That succeeded, and when I visited the server "view" page again, I get a message saying that the module can see the collection:

The Solr collection could be accessed (latency: 11.675165725708 ms).

Ok Great! Time to upload the configset!
I click the Upload configset button and say yes to overwrite the _default config set for my collection.
And then I get this error in search_api_solr:

Error message
The configs of the essential Solr field types are missing or broken for server "solr".
You are using an incompatible Solr schema. Please follow the instructions described in the README.md file for setting up Solr.

Looking in the logs on the solr server, I see:

solrconfig.xml:  is no longer supported, use solr.xml:/metrics/reporter section instead

I don't know how to interpret this but it sure seems like there is an incompatibility between the config set search_api_solr wants to generate and what solr-cloud is expecting.
Well, let's try with drush:

% ddev drush @adobe --numShards=1 search-api-solr:upload-configset solr
In SolrConfigSetController.php line 336:
The configs of the essential Solr field types are missing or broken for server "solr".
Failed to run drush @adobe --numShards=1 search-api-solr:upload-configset solr: exit status 1

I'm stumped. I tore down and removed my ddev setup, purged my volumes and docker images and tried again. Same result.

Just wiped everything, including docker volumes and images and went through all the steps in the README, exactly as specified.
I think this is the source of all the problems. When I attempt to upload my config set (either via drush or via the UI), the following error message is returned:

The configs of the essential Solr field types are missing or broken for server "solr".

I interpret this to mean, given that other users seem to be having success with this ddev add-on, that something has changed in the solr docker container and ddev-solr needs to supply or run a setup command to install the essential solr field types.
% docker --version
Docker version 25.0.3, build 4debf41
% ddev --version
ddev version v1.22.7
search_api_solr module version: 4.3.2
search_api_solr_admin module is enabled.

You're doing ddev drush @adobe ? Doesn't that mean that this entire problem is happening on the remote server and has nothing to do with this add-on?

Thanks @rfay, but no, "adobe" is just my drush alias for the drupal site in the ddev project I am working on. Nothing is happening with a remote server. I have removed it from the error description in order to remove confusion, but I do wonder if this add on has been tested in multi-site setups. The old ddev-drupal9-solr add works fine in multi-site mode.
One thing I appreciate about this add-on (ddev-solr) is that it doesn't force you to name your "core" (now collection) any one specific thing. But the cost of this seems to be that since nothing is pre-built/pre-configured on the server, that there are more problems in getting to a non-error state. When I ran into the error with the missing basic fields config, I wondered if it could be a change in the upstream docker container ... that this add on used to work but now is assuming certain defaults and configuration that no longer holds.

The old ddev-drupal9-solr add works fine in multi-site mode

I guess there's nothing wrong with using that one.

I do wonder if this add on has been tested in multi-site setups

I have not heard of anyone using ddev-solr with multi-site, but don't know why that would actually be relevant.

this add on used to work but now is assuming certain defaults and configuration that no longer holds

There have been no non-trivial changes to this add-on in 10 months, see https://github.com/ddev/ddev-solr/commits/main/
Please try it in a limited test environment and see if you can sort out what's going on. (Create a trivial project using a quickstart, extend it to a multisite after you have the add-on working, etc.)

Some interesting reading that might have some connection to the issues encountered here:

DDEV Solr Cloud Instructions do not work: no collection created
entity type needs to be installed - Cache, Request Dispatcher, Request Handler

Was https://www.drupal.org/project/search_api_solr/issues/3104189 actually the issue for you?

Was https://www.drupal.org/project/search_api_solr/issues/3104189 actually the issue for you?

Not sure. I am waiting for another time window where I can dig deeper and hopefully come up with some useful knowledge to help others.

@apotek try to follow the README with a fresh installation of drupal. It seems that you messed up your Solr configs in Drupal.
If that works we can take a closer look.

no further feedback

no further feedback

Sorry for the lack of feedback. I didn't have time budget to keep troubleshooting this add-on. We ended up having to use https://github.com/ddev/ddev-drupal-solr in order to get things working.

Pardon me commenting on a closed issue, but it is the same error.
The readme in https://github.com/ddev/ddev-drupal-solr  urges the use of this add on instead. I had initially tried that add on, but saw that note, so trying to use this one, but I am seeing the same errors as apotek originally reported.
I was posting in this thread in Drupal slack https://drupal.slack.com/archives/C3E9QDZ5M/p1720043648456549 about it.
When I try to "Upload and create collection", I get

org.apache.solr.core.SolrCoreInitializationException: SolrCore 'dev' is not available due to init failure:
...
Caused by: org.apache.solr.common.SolrException:
...
Caused by: org.apache.solr.common.SolrException: No coreNodeName for CoreDescriptor[name=dev;instanceDir=/var/solr/data/dev]

Even when running the commands directly in the solr container, I create a collection, and I still run into errors using

What happens if you delete the docker solr volume?

I think the fix was to simply update to the latest Search API Solr module in Drupal. I had thought I was on the latest version, but after checking, I was on 4.3.2. Updating to 4.3.4 seems to fix the error when uploading the config set.
Sorry to bug you!

Thanks! As you discovered, it's always great to make sure latest stable "everything" before asking for support.


Fix batch implementation logic

What does this PR do?
Fixes the batch implementation logic. Before this fix, we didn't reset the query_spec object at each loop run, so we ended up doing total_nb_objects / batch_size requests, each querying metrics for every objects instead of just the objects of the batch.
Motivation
What inspired you to submit this pull request?
Review checklist

[ ] PR has a meaningful title or PR has the no-changelog label attached
[ ] Feature or bugfix has tests
[ ] Git history is clean
[ ] If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

Additional Notes
Anything else we should know when reviewing?

Codecov Report

Merging #2265 into master will decrease coverage by 4.48%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2265      +/-   ##
==========================================
- Coverage    86.1%   81.62%   -4.49%
==========================================
  Files         189       15     -174
  Lines       13639     1219   -12420
  Branches     1442      134    -1308
==========================================
- Hits        11744      995   -10749
+ Misses       1488      193    -1295
+ Partials      407       31     -376


Documentation restructure

The documentation has been reorganised to better allow distinct kinds of documentation, with distinct audiences, to be written.
This involves splitting the documentation into several distinct sections based on how it should be written. This is Tutorials, How-to guides, References, and Explanations.

Tutorials are for people learning their way around.
How-to guides answer the "how?".
References answer the "what?".
Explanations answer the "why?".

This is a commonly used, and well regarded documentations style used by projects such as NumPy.
More information can be found here: https://documentation.divio.com/

This PR just restructures the documentation. It will be an ongoing effort to expand and update the documentation.

To aid review, here is a rendered version of the documentation.


refactor: convert handle-accessibility to typescript

I'll have a closer look at the test file when you had a look at the comments and updated

Not sure what went wrong... I expected git 'files changing part' keeps the transitions from js to ts for reviewing but I see now the js file is totally deleted  and a new file is replaced, sorry for this destruction 😣

Not sure what went wrong... I expected git 'files changing part' keeps the transitions from js to ts for reviewing but I see now the js file is totally deleted and a new file is replaced, sorry for this destruction 😣

That is fine, it has happened to at least one of my PRs. I think it is when you've changed too much it doesn't consider it a rename anymore


[Suggestion] Style the lyrics view.

Please add the possibility to patch lyrics colors too, similar to windows's version where it's dark background and green text.  I wonder why Spotify doesn't patch it itself.
Thank you :D

At this time, UI customization is not something I wish to entertain with this project. Outside of hiding podcasts and episodes on the homescreen, nothing else with the UI is customized unless it's an official feature/setting.. and the only reason the podcast hiding is included is because it's a simple one-liner patch (for now anyway).
I may reconsider this in the future, possible just removing color from lyrics background so it's always black background with grey/white letters... but picking and choosing colors is unlikely.
Spicetify is always an option for those who wish to theme and customize the UI.

@aleeeee1
I've added a new commandline argument to set lyrics background color to black, -l or --lyricsnocolor will trigger this.
The patch is targeting Spotify builds v<IP_ADDRESS>5 and up although only builds 1.2.19+ have been tested so far.
Users should consider this feature experimental until I have time to test more builds and see what new builds released in the future will look like.
Option flag has been added into the README "Options" section, but will not be considered a permanent feature until the -l option in the README gets a hyperlink pointing to the FAQ with a proper explanation.
I will not be adding lyric color customization.  Users wanting UI customization and color theming should stick to Spicetify.


Error: Peer certificate cannot be authenticated with given CA certificates

Insomnia Version: 6.6.2
Operating System: Windows 10 on Desktop PC

When I am trying to access an SSL endpoint on my private domain, I see the error in the title.
Perhaps the error goes away if I uncheck "Validate certificates" in "Preferences", but I do not want to do that. I want to use SSL verification.
There is ZERO information about why Insomnia does not simply read the global certificate store, or where in the world it looks for root CA certificates on Windows.
If it looked in the global Windows certificate store t would have found a shiny trusted root certificate (self signed) with my private domain name in it.
So my question is: how do I direct Insomnia to read my private root CA, and therefore I do not need to disable "Validate certificates" in Preferences?

On Windows, Insomnia uses a bundled CA certificate that is included in the build.
There's no way to define your own at the moment or to use Window's global cert store. For now, disabling SSL validation is the only option.
There's an open issue to improve support for this in the future: #675

Is Insomnia holding any copy of the certificate? The server certificate has expired, it has been replaced by the current one and I am getting this error message even though everything has returned to normal in the regular browser. There was no problem before the certificate expired.

@xorock Currently facing the same issue. What was your solution?


error handler function added

Thanks!

Hi,
Thanks for this very useful library.
I added below line to my composer.json;
"himiklab/yii2-easy-thumbnail-image-helper" : "*",
composer update command downloaded automatically version 1.1.
In this version 'EasyThumbnailImage' class doesn't contain 'errorHandler' method.
Each time I make composer update, I overwrite class contents. I require your help on this.
Thanks in advance.
Berk.

https://github.com/himiklab/yii2-easy-thumbnail-image-helper/releases/tag/1.0.2


upgrade version in CI

:tada: This PR is included in version 1.4.0 :tada:
The release is available on:

GitHub release
v1.4.0

Your semantic-release bot :package::rocket:


[Release 1.22] Remove kube-ipvs0 interface when cleaning up

Backport: https://github.com/k3s-io/k3s/issues/5643

Followed the same steps as in https://github.com/k3s-io/k3s/issues/5643#issuecomment-1156905123 but validated using v1.22.10+k3s1


Durations as units of time

I'd love to be able to use durations as units of time in other methods. This is mostly a convenience thing, but it would help when working with APIs that insist on returning time intervals as strings.
const t1 = moment('2020-01-01T00:00:00')
const t2 = moment('2020-01-01T12:00:00')
const t3 = moment('2020-01-01T12:15:00')
const hour = moment.duration('01:00:00')

t2.diff(t1, hour) // returns 12
t2.isSame(t3, hour) // returns true

This could be only for durations that are exactly some unit of time, or for any duration that humanizes to one unit of time, i.e. moment.duration('01:00:01') counts as 'an hour.' Or methods like diff and isSame could take a number of milliseconds as well as a string (seems harder to implement).

Thanks for opening this request.
For now, we will not take this change.
Perhaps this would be best as a plugin for Moment.js.
https://momentjs.com/docs/#/plugins/
If you do work out a solution, you could post it here, or make a PR to https://github.com/moment/momentjs.com/ .
Best of luck.


Poll: switch to create-react-app

Dear followers,
I've been thinking about migrating the project to create-react-app.
Pros:
The frontend build dependencies would be managed by a facebook maintained project
This means that the project will follow all the good practice identified by the community more or less automatically
Cons:
It would hide the nitty-gritty details of the build system from you
Please vote with 👍 in favor of CRA and 👎  if you want to keep the build as-is.
You're welcome to comment if you want to give more insights.

I'm a long-time Sencha ExtJS developer and found trying to learn everything at once with React is tough so I decided to step back and start with CRA. Your project is great because we use Spring Boot and Docker for microservices. It would be nice to see if you can pull off all the functionality (proxy, advanced webpack, etc) you currently have using CRA without "ejecting".


Has there been any progress?

Wanted to check in here and ask: has there been any progress made on this? As Node.js is growing, I'm seeing more and more that we need an updated site - community efforts, foundation efforts, and general optimizations are all growing, and an updated site would be extremely helpful in streamlining these efforts.

Sadly, I haven't yet managed to tackle this. We're still on a big project that needs to be released first!

@leo Sorry to bother you, but I'm hoping Now Desktop 2 was the big release? 😁
We briefly discussed this in today's Community Committee meeting - given how long it's been, do you think this project will be moving forward?

Yep, it's done now. @evilrabbit and I will collect all the information tomorrow and start working on the fresh layout together... 😊

@leo Awesome. I know that both the Website WG (@nodejs/website) and (@nodejs/commcomm-members) are interested in this project, and are keen to get started. That said, there may need to be some further co-ordination now that Mikeal is not at the Node.js Foundation anymore 🤔

It would be nice if you could keep me in the loop. We have some design related issues in the website project, and it would be helpful for us to coordinate the general direction we're moving in.

@bnb

Do you have a milestone for it?
Who will manage the project?


Change getting the fundle source from wget to curl.

Minimalist RetHat system and OSX don't contain wget in default.

Great, thanks!


Allow non-core taps to create bintray packages

Normal bintray users need to specify licenses and vcs_url fields when creating packages.
I think HOMEBREW_BOTTLE_DOMAIN can be replaced by the url I provided, cause it works for everyone, "homebrew", "linuxbrew" and others.

This has bit me before... I think better messaging is a partial solution here with respect to the license field. Or, if we ever do merge the licenses project that would be an out as well. Agree with Mike that we shouldn't be setting these for the user.

Closing, cause Bintray said we should not set static values as license and vcs url.

Supporting storing bottles as GitHub release artifacts for third-party taps could be a viable workaround, if someone sufficiently motivated wanted to do the work.


Get List started for linux boot managers

https://ramsdenj.com/2018/05/29/zedenv-zfs-boot-environment-manager.html
https://github.com/b333z/beadm

not sure how to create a directory to get this started

incorrect approch
need to upload a file


[feature request] CSV like output

Type
CLI
Suggestion
I'm trying to do an script that handle series I'm watching and check new episodes on one command. I had something that worked really great with yt-dlp but reading the output of this software is a nightmare. Because of that I would humble ask if there is a possibility to dump search results based on alias like it's done in filename scheme.
for example: I'd like to check if new episodes of a season is available. So I pass the anidl somewhat like this:
anidl --service crunchy --series [code] --file-dump $alias1 $alias2 $etc --output-file-name name.csv
a csv file is a lot easier to parse and I could do something that could use this to easely get info around with one click.
A way to do the same that produce an easy to read by machine would help a lot too.
Thanks!

True, that would be a nice feature. Though we actually have something like this built in called the archive. Using --addArchive, you can save every series you download to the archive, so when you run --downloadArchive with --but it will download all the new episodes of a show. I suspect a lot of refactoring would have to be done to output as a csv, json might be a little more manageable though

True, that would be a nice feature. Though we actually have something like this built in called the archive. Using --addArchive, you can save every series you download to the archive, so when you run --downloadArchive with --but it will download all the new episodes of a show.
As for the actual output, I suspect a lot of refactoring would have to be done to output as a csv, json might be a little more manageable though

json is still easier and would acomplish the objective completly. There's no need of CSV if a good json is outputed.
about --downloadArchive, I was testing it and didn't understand it entirely just with documentation. When I add a series anidl tries to download everything... so I really don't know if it's possible to add for example just download the dub version with the .ass subs for that dub (not the complete one). So I don't think how I could use it to that without download everything else.

So I was thinking of this over dinner, and would the raw JSON data from the API work (This would be the easiest to accomplish)? Or do you want something more custom? Is there a structure you would prefer? In the example you gave (series), I was thinking something like this:
{
  "series": {rawCrunchySeries},
  "seasons": [{rawCrunchySeason}, {rawCrunchySeason}]
}

In the seasons would be the episodes from the API. Honestly making a full TypeScript implementation of their API may not be a bad idea - but a bit off topic.
Circling back to the archive though, basically everything you download (so long as you have --addArchive in the command or addArchive: true) will be added to the archive. It doesn't really handle multiple languages though, you could specify the dub you want to download with --dubLang when using --downloadArchive, but, it would be added to the archive if addArchive is present, preventing a download in another dub at another time. One day I have to completely rewrite how archiving works, but I haven't gotten around to it yet.

It doesn't need to be custom. If it's easier to just drop the json from the api for you then that's enough for me too. All info will come from there anyway and that can potentialy opens development for stand alone GUIs that can hook info from this program if a dump of the json can be made for any input.


A couple of feature requests

Choose the register that the icon is copied to
Choose the size of the popup window
A keybinding to reset the search without having to close the window
Allow NerdIcons to take an optional parameter which would start the search before the popup opens
A keybinding to move a page up/down at a time

Editing to add:
6. Allow searching by a glob/regex type pattern
If I have some time today, I'll try and implement some of these as well.

use 0 register is fine i think.
now you can config the width in options
idk what does this mean. press <CR> to search then it will clean the search keyword in prompt buffer you don't need close window
support you can use NerdIcons keyword
no need just move down move up is enough?
you can use the lua regex as keyword


build.zig: Merge src/build.zig to build.zig

I see no reason to have to keep multiple build.zig files in the same project unless the project is a monorepo with multiple independent components. Redundant with #4387.

[x] Moved src/build.zig to build.zig
[ ] Should work with https://github.com/rstefanic/raylib-zig-build-bug @rstefanic
[ ] Should work with https://github.com/Yerdun/raylib-zig-dep-bug @Yerdurn
[ ] Consider rethinking Raygui support? Could simplify lots of things
[ ] Consider if it really is worth applying the default values of src/config.h when -Dconfig is specified

I think it should work. Would appreciate if you two could test if it works with the lines using Path(.{ .cwd_relative = commented out. I'm on NixOS and I could successfully compile without those lines except for when targeting wasm32-emscripten or wasm64-emscripten.

Breaking changes:

RaylibOptions introduced in https://github.com/raysan5/raylib/pull/4375 will become Options
No-longer tries to look into src/config.h to apply possible default configs when passing -Dconfig. This comes with the benefit of being able to supply more than one space-separated flags now

Hi, still have yet to test with your commits or read up on your changes, but I did want to address something you mentioned:

Consider if it really is worth applying the default values of src/config.h when -Dconfig is specified

I think it should. To my understanding, this is done to maintain identical functionality with the other build systems: when provided with custom cflags, they keep all the default values, but overwrite them with any changes should the user supply any. That said, (a) I have not actually looked through the other build scripts and thus cannot confirm, and (b) I do not know if doing this in build.zig is especially cumbersome.

No-longer tries to look into src/config.h to apply possible default configs when passing -Dconfig. This comes with the benefit of being able to supply more than one space-separated flags now

In 4a6a7511d1c2cfff19b506735d9f6a207ca9eea2, would replacing the original line 94 with new lines 71-74 work? I'm thinking your idea of adding each flag individually to raylib_flags_arr could integrate into the current way it's set up, thus fixing the problem while not completely rewriting its structure. Haven't tested it yet, but just an idea.
I tried adding a comment to your commit to hopefully demonstrate the idea more clearly.

@Yerdun, care checking https://github.com/raysan5/raylib/pull/4393/commits/3e7b5a0bf40096acc12aca794a576e7af5e60dde?
I "fixed" the situation by making -Dconfig take a []const []const u8 instead of a []const u8.
Here's roughly how I'm testing stuff:
    const raylib = b.dependency("raylib", .{
        .target = target,
        .optimize = optimize,
        .config = @as([]const u8, "-Werror -Wextra"),
    });

    const exe = <omitted>
    exe.linkLibrary(raylib.artifact("raylib"));

The code itself is quite ugly. I'm assuming there is no way to properly pass []const []const u8 with a length over 1 using std.Build.option, but I would more than love to be wrong.

@sagehane In my experience, PRs with a big list of TODOs and changes do not end-up well... They usually generate more issues and more PRs and breaking things... If this PR is just trying to solve the two build.zig files, I'm ok with keeping both.

Okay so, the main motivation for moving src/build.zig was to simplify the logic such that it would hopefully be easier to maintain the file. I am strongly in favour of doing this, but I'm not sure everyone agrees.
And, sort of as a result, I did find out that there's a bug related to -Dconfig and a way to sort of address it. I think this part is more important.
So should I:

Fix the -Dconfig issue first with a different PR and defer whether or not src/build.zig should be merged to root?
Just merge this (after addressing conflicts) and get both?

Regarding the code comments with TODOs, they are (bar two locations where I questioned the necessity of the code) about documenting unfortunate hacks that would probably be better without it. All functionality should work as intended for the end user and it should only be a consideration for future maintainers of the file.

@Yerdun, care checking 3e7b5a0?

Sorry to get back late, but can confirm my sample repo compiles just fine now. The raylib dependency takes custom cflags into account, even when cross-compiling to Windows. I also tried the other repo configured for 0.13.0 and that one compiles too.
Also, for what it's worth, I briefly looked over your changes and I think your code makes sense. Then again, I'm just an end user who's really bad at Zig, not a maintainer.
Thanks a lot for your hard work!

I did find out that there's a bug related to -Dconfig and a way to sort of address it. I think this part is more important.
So should I:
1. Fix the `-Dconfig` issue first with a different PR and postpone the decision of whether or not `src/build.zig` should be merged to root?

2. Just merge this (after addressing conflicts) and get both?

May I ask what the problem with -Dconfig is? Also, I would personally be in favor of the first option, as I prefer to split steps whenever possible. But again, I'm not a maintainer.

The only thing I'd do differently is use NOTEs instead of TODOs, since I think that tag more accurately describes your comments.

Sure, my coding conventions might look weird to some people and I'm perfectly willing to conform if it's an issue.

May I ask what the problem with -Dconfig is?

1. Broken when defining the .config field of std.build.dependency
I introduced this. The biggest issue.
Before:
https://github.com/raysan5/raylib/blob/c4be01329493d0e772a04de08639b4da32905d2a/src/build.zig#L67-L71
Now:
https://github.com/raysan5/raylib/blob/dc5e6e0ad089f8eda932efb4646d6bbafe0713cc/src/build.zig#L88-L90
Currently, the wrong path is passed to readFileAlloc. The fix is probably to revert to the old code or use the std.build.pathFromRoot like in this PR.
2. The code for checking what flags are user-specified is faulty
Was like this when I got here.
https://github.com/raysan5/raylib/blob/dc5e6e0ad089f8eda932efb4646d6bbafe0713cc/src/build.zig#L99-L109
Say config.h defines options foo and foobar, and I pass in -Dfoobar. Then std.mem.containsAtLeast will return true for both foo and foobar, thus unsetting both.
A real example might be if I passed a value for -DSUPPORT_MODULE_RTEXTURES. As-is, the current code would also unset -DSUPPORT_MODULE_RTEXT as  both share the exact same prefix:
std.mem.containsAtLeast(u8, "-DSUPPORT_MODULE_RTEXTURES", 1, "-DSUPPORT_MODULE_RTEXTURES") == true;
std.mem.containsAtLeast(u8, "-DSUPPORT_MODULE_RTEXTURES", 1, "-DSUPPORT_MODULE_RTEXT") == true;

This PR solves this by checking if the user-specified flag is equal to the flag or if it's immediately followed by an equal sign.
3. No way to pass more than one user-specified flag
Was like this when I got here. The solution isn't completely pretty but I'm convinced it's worthwhile.
https://github.com/raysan5/raylib/blob/dc5e6e0ad089f8eda932efb4646d6bbafe0713cc/src/build.zig#L112-L113
Say I pass "-Dfoo -Dbar" as a user-specified config. Then the build system literally tries to apply that instead of applying "-Dfoo" and "-Dbar" separately, causing the flag to be ineffective. One can easily test this by seeing if compiling the project with -Dconfig="-Werror -Wall" will successfully result in a build error, for example.
This PR solves this by splitting on the value when encountering a whitespace, and converting it to []const []const u8 (slices of strings). This is technically problematic too because now flags like -x c++ wouldn't be passed properly. But Zig currently doesn't support that flag and even if it did, -xc++ is also valid afaik. I don't know enough about C to know what other flags could be problematic.
4. It's tedious to pass multiple user-specified flags through std.build.dependency
Related to 3, not a bug but ergonomics issue. This has weird implications and it could be better.
This PR solves this by changing the type of Options.config from []const u8 (strings) to []const []const u8 (slices of strings).
From the dependency side of stuff, I can now do:
    const raylib = b.dependency("raylib", .{
        .target = target,
        .optimize = optimize,
        .config = @as([]const []const u8, &.{
            "-DSUPPORT_MODULE_RTEXTURES=1",
            "-DSUPPORT_MODULE_RTEXT=1",
        }),
    });

Instead of having to do:
    const raylib = b.dependency("raylib", .{
        .target = target,
        .optimize = optimize,
        .config = @as([]const u8, "-DSUPPORT_MODULE_RTEXTURES=0 -DSUPPORT_MODULE_RTEXT=0"),
    });

It looks much better as the number of flags get longer. But I could also just expose a function that converts a list of strings into one big space-separated string. Would also have the same -x c++-like issues. (Perhaps this is better, but would likely be more confusing to those unwilling to read
the documentation of the build.zig... But that's already kind of the case because how else would people know to have to use @as([]const []const u8, ...) to satisfy the type of the .config field?)
The implication is that passing -Dconfig="-Dfoo -Dbar" through the command-line will now be treated as @as([]const []const u8, &.{ "-Dfoo -Dbar" }). I'm not aware of any way to pass it as a non-singleton value. I hope upstream addresses this, might ask them later. Anyhow, I have a logic that checks if the length of the slice is 1 and converts it into a singleton list of space-separated flags. I think this is quite hacky so now I'm really reconsidering the "expose a helper function" method.

Thanks for the detailed rundown. Speaking as a layman end-user, I stand behind your proposed solutions. Even though you consider some hacky, they do solve the immediate issues at hand, and I can't think of any additional problems they would inadvertently introduce.

So should I:
1. Fix the `-Dconfig` issue first with a different PR and postpone the decision of whether or not `src/build.zig` should be merged to root?

2. Just merge this (after addressing conflicts) and get both?

I'm also going to change my stance on this, I would go with #2. You've already gotten things working, and retrofitting your changes to the old setup would just be unnecessary labor. Your current setup also has the added benefit of fixing the build.zig.zon dependency problem.
That said, given my inexperience, others' input here would have more weight than mine. And ultimately, the final decision on how to handle this is up to Ray.

+1, it'll be easier to follow the commit history if this was 2 PRs, one for the compile logic changes and one for relocating src/build.zig. Trying to see what actually changed inside the build script is not trivial when it's coupled with the commit moving it into the project root

@sagehane Should this PR remain open conseidering #4398 has been merged?

@raysan5, yes, please keep it open. I do plan to merge src/build.zig as some use-cases are still broken. Unless you prefer me making a new PR just for the sake of it.

I think this is ready for review. I got all the bug-reproduction repos working with Zig versions 0.12.1, 0.13.0, and the master release of Zig.

In the meanwhile, I also noticed another strange thing... I am thinking of making a PR for https://github.com/sagehane/raylib/commit/f89c637b9e3d957c850746b9e2262985fe9bd51d and I would like testing from some testing on Linux users that aren't on NixOS.
The lines where I removed a bunch of linkSystemLibrary is insignificant as Zig's linker is apparently smart enough to unlink libraries if they're not being used. If those lines are needed for some people's setup, it can trivially be added back. I just wanted to see how much I can remove whilst getting everything to build.
The interesting parts are:

Trying to add /usr/include and /usr/lib to system paths when Zig should be doing that by default
Trying to link drm instead of libdrm and hacking through it by adding /usr/include/libdrm

This breaks builds on NixOS with -Dplatform=drm as the path doesn't exist on those systems

Not properly documenting the fact that pkg-config is a dependency for some of these libraries

Maybe that's just a packaging quirk on NixOS and it works fine elsewhere

Anyhow, I'll worry about that PR if/when this one gets merged.

In the meanwhile, I also noticed another strange thing... I am thinking of making a PR for sagehane@f89c637 and I would like testing from some testing on Linux users that aren't on NixOS.

With this commit, I confirm I was able to successfully build raylib and the example projects on Zig 0.12, 0.13, and master. For the record, I used the "Both" and "X11" windowing backend options, as well as building on the drm platform.
I also briefly tested this commit on my bug reproduction repo, on 0.12. That worked fine, too.

@sagehane Thank you very much for all the work put into this improvement/redesign! Merging!


Uncaught TypeError: Cannot read property 'buffer' of undefined

[Enter steps to reproduce below:]

Testing out joining a pairing session through HipChat to myself
After asking for the @mention name is threw the error - but still sent to HipChat

Atom Version: 0.184.0
System: Mac OS X 10.10.3
Thrown From: atom-pair package, v0.37.0
Stack Trace
Uncaught TypeError: Cannot read property 'buffer' of undefined
At /Users/aaron/.atom/packages/atom-pair/lib/atom_pair.coffee:162

TypeError: Cannot read property 'buffer' of undefined
  at Object.module.exports.AtomPair.startPairing (/Users/aaron/.atom/packages/atom-pair/lib/atom_pair.coffee:162:31)
  at /Users/aaron/.atom/packages/atom-pair/lib/modules/hipchat_invite.coffee:55:10
  at IncomingMessage.<anonymous> (/Users/aaron/.atom/packages/atom-pair/node_modules/node-hipchat/lib/hipchat.js:226:24)
  at IncomingMessage.emit (events.js:138:20)
  at _stream_readable.js:909:16
  at process._tickCallback (node.js:364:11)

Commands
     -6:23.9 window:increase-font-size (div.settings-view.pane-item)
  3x -6:23.5 settings-view:open (div.settings-view.pane-item)
     -2:16.5 core:paste (atom-text-editor#atom-pair.hipchat_token.editor.mini)
  6x -2:14.1 core:backspace (atom-text-editor#atom-pair.hipchat_room_name.editor.mini)
     -1:47.6 command-palette:toggle (atom-text-editor.editor)
  2x -1:46.2 core:backspace (atom-text-editor.editor.mini)
     -1:45.2 core:select-all (atom-text-editor.editor.mini)
  3x -1:45.0 core:backspace (atom-text-editor.editor.mini)
  2x -1:36.1 core:move-down (atom-text-editor.editor.mini)
     -1:35.2 core:confirm (atom-text-editor.editor.mini)
     -1:35.2 AtomPair:invite over hipchat (atom-text-editor.editor)
  6x -1:29.6 core:backspace (atom-text-editor.editor)
     -0:01.8 core:confirm (atom-text-editor.editor.mini)

Config
{
  "core": {},
  "atom-pair": {
    "hipchat_token": "4407a1d5f54e3a5e9bce1751d8754c",
    "hipchat_room_name": "Uidaho"
  }
}

Installed Packages
# User
atom-pair, v0.37.0
autocomplete-plus, v2.1.6
minimap, v4.2.2

# Dev
No dev packages

What was the resolution to this?  I'm trying to integrate Slack and it's throwing the same error for me.


api: enhance v3 CommonTlsContext for agentless support

Signed-off-by: Sanjay Pujare<EMAIL_ADDRESS>For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Commit Message: api: enhance v3 CommonTlsContext for agentless support
Additional Description: add protos/fields needed to support agentless (without SDS) secret access
Risk Level: low
Testing: pre-submit and do_ci fix format checks
Docs Changes: none
Release Notes: -NA-
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
@htuch @costinm @markdroth  PR for agentless secrets as discussed.

Can you please elaborate on the description of this change? It sounds like there was a previous conversation related to this, but for people (such as myself) that weren't part of the conversation, it isn't clear what the purpose of this change is.

Can you please elaborate on the description of this change? It sounds like there was a previous conversation related to this, but for people (such as myself) that weren't part of the conversation, it isn't clear what the purpose of this change is.

Sure. gRPC recently added xDS support for load balancing and other routing features. This enables users to create proxyless service meshes. However for transport security (TLS/mTLS etc) the xDS protocol assumes the presence of SDS servers (for example Istio Agent or Node agent). To make the solution agentless as well as proxyless we are considering an SDS-server-less approach where we bypass the SDS server and have a CA-client in gRPC directly to talk to a service mesh CA to acquire certificates. This PR is related to that. Hope that answers your question.

Thank you, that's very helpful context.
So is this logically similar to talking to an SDS server, but with a different, previously existing/implemented protocol?

Thank you, that's very helpful context.
So is this logically similar to talking to an SDS server, but with a different, previously existing/implemented protocol?

In case of SDS, the SDS server creates the CSR and returns cert + private key to the client (Envoy) . With the CA client approach, the client (Envoy) creates the CSR so private key is not shared with anybody.

I think this PR is premature.  I don't think we have enough consensus yet on exactly how this should work.  I think we need to finalize our design before we add anything to the xDS API.

Agreed with @markdroth; can we have a public design document describing how this will work? Thanks.

Agreed with @markdroth; can we have a public design document describing how this will work? Thanks.

Sure, I'll create a design doc and attach to this PR. Hopefully that will help us reach broad  consensus.

FWIW I'm definitely in favor of doing this change. I have been thinking about it for a while. It will make it much easier to natively implement ACME (https://github.com/envoyproxy/envoy/issues/96) as an extension. We need to think about async reload support for these extensions also, so there is some plumbing overlap with SDS.

FWIW I'm definitely in favor of doing this change. I have been thinking about it for a while. It will make it much easier to natively implement ACME (#96) as an extension. We need to think about async reload support for these extensions also, so there is some plumbing overlap with SDS.

@mattklein123 thanks. Can you explain what you mean by "async reload" support and "plumbing overlap with SDS" in this context? We have used "async reload" at the transport/handshake level where a cert is acquired dynamically and asynchronously for a new (m)TLS connection. But this is implied in xDS via the *TlsContext fields for clusters/listeners and that semantic is not changed by this proposal.

Can you explain what you mean by "async reload" support and "plumbing overlap with SDS" in this context?

Sorry I just meant that we need to make sure that the Envoy side implementation supports async fetch from the extension. This would allow certificates to be fetched over the network, refreshed, etc., much like how SDS works.
Optimally, SDS would be implemented in a future version as an extension, but I think that is out of scope for this set of changes.

Is there a preferred format for a design doc? Is Google doc okay?

The requested design doc is here https://docs.google.com/document/d/1tZjwXz6vBpYF5NN6qajKyFM8UvgUyAcJd79mvip5Cq0/edit?usp=sharing .

Please make the doc world-readable.

Please make the doc world-readable.

Just did. I had to make a copy to do that. I edited my comment to include the new link.

@sanjaypujare sorry can you please make it world commentable?

@sanjaypujare sorry can you please make it world commentable?

Done.

/wait

/wait

I notice this added the "waiting" label - curious what it signifies.

/wait

I notice this added the "waiting" label - curious what it signifies.

It allows filtering PRs by label. PRs with the waiting label are waiting for something and so they don't need immediate review.
Pushing a commit automatically clears the label.
In this case, since the current work is happening in the linked design doc, I added the waiting label here.

@htuch and others: I have addressed your comments in the design doc as well as in the PR.

I see the following error from CircleCI
envoy/extensions/transport_sockets/tls/v4alpha/tls.proto:153:5: Fields in oneofs must not have labels (required / optional / repeated).

which is because of the following (generated) v4alpha tls.proto snippet field tls_certificate_sds_secret_configs:
  oneof dynamic_tls_certificate {
    // Configs for fetching TLS certificates via SDS API.
    repeated SdsSecretConfig tls_certificate_sds_secret_configs = 6
        [(validate.rules).repeated = {max_items: 1}];

    // Config for fetching TLS certificates via CA client plugin.
    // [#not-implemented-hide:]
    CAClientPluginConfig tls_certificate_ca_client_plugin = 9;
  }

Questions:

why is it a repeated field if max_items value is 1? If it is because of legacy reasons can we remove the repeated attribute for this field now?
why didn't this failure happen in other CI tests or even my own local build (./ci/run_envoy_docker.sh './ci/do_ci.sh fix_format'). Or should I ignore this error?

Re: the CI errors. It's right that repeated can't be in oneof. Not sure why tls_certificate_sds_secret_configs is repeated but size bounded to a singleton; maybe that reflects current implementation but might change in the future?
In any case, I don't think we can oneof promote in this case.

As I also mentioned elsewhere, I think we should do a reasonable sanity check that this config would allow us to implement an ACME plugin as well.

Typically when an ACME agent is installed, a key pair is generated for use by the agent and the CA. This pair is called as “authorization keys”. In addition to these keys, the agent has as part of its configuration:

domain(s) to manage
CA to use (i.e. CA's address)
a nonce to use with the CA

To incorporate this into our plugin architecture, I can think of an out-of-band installation process for the ACME agent which will generate the above configuration values. The agent can now function as a plugin and the same configuration can be provided from the control plane (or provided locally for sensitive values like the authorization keys) thru the  CAClientPluginConfig proto.
I'll add the above to the design doc I created for this PR.

To incorporate this into our plugin architecture, I can think of an out-of-band installation process for the ACME agent which will generate the above configuration values. The agent can now function as a plugin and the same configuration can be provided from the control plane (or provided locally for sensitive values like the authorization keys) thru the CAClientPluginConfig proto.

Yes this is exactly what I was thinking. In a minimal case the plugin can have config to load the various files from its config/disk/etc. As long as you think this possible with the current design this sounds great! Thank you for investigating this. Having ACME support builtin to Envoy is a hugely requested feature and this is a great step in that direction.

I have addressed all the outstanding comments. Hopefully this can be merged soon?

To incorporate this into our plugin architecture, I can think of an out-of-band installation process for the ACME agent which will generate the above configuration values. The agent can now function as a plugin and the same configuration can be provided from the control plane (or provided locally for sensitive values like the authorization keys) thru the CAClientPluginConfig proto.

How can the same config be specified both locally and from the control plane?  Are you proposing some sort of merge semantics?  Or are you just saying that any given deployment would have to choose between specifying the config locally vs. getting it from the control plane?

To incorporate this into our plugin architecture, I can think of an out-of-band installation process for the ACME agent which will generate the above configuration values. The agent can now function as a plugin and the same configuration can be provided from the control plane (or provided locally for sensitive values like the authorization keys) thru the CAClientPluginConfig proto.

How can the same config be specified both locally and from the control plane? Are you proposing some sort of merge semantics? Or are you just saying that any given deployment would have to choose between specifying the config locally vs. getting it from the control plane?

Good question. yes, I am proposing some kind of transfer semantics or merge semantics based on how ACME agents typically work. Things like "authorization keys" result from the ACME agent installation involving some out-of-band verification (such as domain ownership etc.). So these authorization keys can be read locally from a local config file but the rest of the config can be pushed from the control plane. Or everything can be transferred to the control plane so the control plane is the single source of config for all ACME plugins.

Good question. yes, I am proposing some kind of transfer semantics or merge semantics based on how ACME agents typically work. Things like "authorization keys" result from the ACME agent installation involving some out-of-band verification (such as domain ownership etc.). So these authorization keys can be read locally from a local config file but the rest of the config can be pushed from the control plane. Or everything can be transferred to the control plane so the control plane is the single source of config for all ACME plugins.

I don't see anything in this PR that suggests how that would actually work.  Can you show what the config proto might look like for an ACME plugin and walk me through an example of how this might be configured such that part of the config is local and the other part comes from the control plane?
Note that if the answer involves protobuf merge semantics, that's not going to work.  We've discussed that many times before, and it's simply not a usable approach for this kind of configuration.

Good question. yes, I am proposing some kind of transfer semantics or merge semantics based on how ACME agents typically work. Things like "authorization keys" result from the ACME agent installation involving some out-of-band verification (such as domain ownership etc.). So these authorization keys can be read locally from a local config file but the rest of the config can be pushed from the control plane. Or everything can be transferred to the control plane so the control plane is the single source of config for all ACME plugins.

I don't see anything in this PR that suggests how that would actually work. Can you show what the config proto might look like for an ACME plugin and walk me through an example of how this might be configured such that part of the config is local and the other part comes from the control plane?
Note that if the answer involves protobuf merge semantics, that's not going to work. We've discussed that many times before, and it's simply not a usable approach for this kind of configuration.

I am definitely not proposing protobuf merge semantics but merge/transfer only as a concept. In terms of what these protos would look like as an example:
what comes from the control plane (as a typed proto but JSON representation):
{
"managedDomains" : ["foo.com"],
"ACME_CA_Address" : "api.myca.com:8080"
}

And then what is loaded from the local file or some secure storage (possibly encrypted):
{
"authorization keys": {
    "private": "...",
    "public": "..."
},
"nonce": "..."
}

The plugin will just get these 2 blobs separately and use them as such (no protobuf merging here).
What I meant by transfer semantic is once the authorization_keys and nonce values are known they could then be added on the control plane config side and the xDS server would then send the whole blob (and not need any local config file):
{
"managedDomains" : ["foo.com"],
"ACME_CA_Address" : "api.myca.com:8080",
"authorization keys": {
    "private": "...",
    "public": "..."
},
"nonce": "..."
}

I am not implying merging or flexibility of either source for a configuration value but a plugin can decide how they want to split the config between the 2 sources. Does it answer your question?

I am definitely not proposing protobuf merge semantics but merge/transfer only as a concept. In terms of what these protos would look like as an example:
what comes from the control plane (as a typed proto but JSON representation):
{
"managedDomains" : ["foo.com"],
"ACME_CA_Address" : "api.myca.com:8080"
}

And then what is loaded from the local file or some secure storage (possibly encrypted):
{
"authorization keys": {
    "private": "...",
    "public": "..."
},
"nonce": "..."
}

The plugin will just get these 2 blobs separately and use them as such (no protobuf merging here).

Is it expected that all ACME plugin instances would use the same authorization keys and nonce, or would this data potentially need to be specified on a per-instance basis?
Would this local data be in the Envoy bootstrap file, or would there be some other file to read?  If there's some other file, how would the plugin instance know which path to read?  Please make sure we have a complete story for this that @htuch and @mattklein123 are happy with.
How would this work in gRPC?  I am definitely not sold on the idea of putting this in the bootstrap file for gRPC.
I'm not seeing a complete story for ACME.  I think this use-case needs to be spelled out somewhere, since ACME is the one plugin that we are certain to have in both Envoy and gRPC.

Is it expected that all ACME plugin instances would use the same authorization keys and nonce, or would this data potentially need to be specified on a per-instance basis?

Each ACME plugin needs to act as a unique ACME agent so cannot share these values. Definitely nonce cannot be shared because of its nature. Also I haven't found much info about using ACME within the service mesh scenario with SPIFFE identities but there is more info about ACME in the traditional HTTP/web scenarios with FQDN identities. In those scenarios in case of a web-farm there is a single ACME agent that is responsible for fetching/refreshing certs and storing them in a shared storage that is then used by all the servers in the farm.

Would this local data be in the Envoy bootstrap file, or would there be some other file to read?

I believe this will have to be a different file (encrypted, with limited access etc).

If there's some other file, how would the plugin instance know which path to read? Please make sure we have a complete story for this that @htuch and @mattklein123 are happy with.

May be part of the plugin config sent by the control plane?

How would this work in gRPC? I am definitely not sold on the idea of putting this in the bootstrap file for gRPC.

Similar to Envoy this will have to be a separate file because of the private key and the path can come from the control plane.

I'm not seeing a complete story for ACME. I think this use-case needs to be spelled out somewhere, since ACME is the one plugin that we are certain to have in both Envoy and gRPC.

Having a complete story for ACME wasn't originally a requirement for this PR although it was mentioned by many. Is it a requirement now?

Having a complete story for ACME wasn't originally a requirement for this PR although it was mentioned by many. Is it a requirement now?

Directly supporting ACME is such a highly requested feature that I think we should at least understand that it can be implemented by this API, obviously not that it should be implemented by anyone doing this plugin work. So yes I think that is a requirement to make sure we don't have to go through a deprecation cycle to handle that case later.
I know very little about ACME quite honestly but my (probably naive) assumption was that the plugin would have whatever embedded config is needed to talk to the ACME server. This data could come encrypted via LDS (itself bootstrapped over SDS or whatever), or possibly via a local file referenced in the config, etc.

FWIW this is the Traefik ACME/Let's Encrypt docs which is probably a good reference of how this might look config wise: https://docs.traefik.io/https/acme/

@sanjaypujare

Is it expected that all ACME plugin instances would use the same authorization keys and nonce, or would this data potentially need to be specified on a per-instance basis?

Each ACME plugin needs to act as a unique ACME agent so cannot share these values.

It sounds like these parameters are per-plugin-instance.  If we store the data in a local file, would we have a separate file for each plugin instance, or would it be a single file containing a map?  If it's a map, what are the map keys going to be, such that each plugin instance can determine which entry to use?

Would this local data be in the Envoy bootstrap file, or would there be some other file to read?

I believe this will have to be a different file (encrypted, with limited access etc).

If it's encrypted, how will the plugin decrypt it?

If there's some other file, how would the plugin instance know which path to read? Please make sure we have a complete story for this that @htuch and @mattklein123 are happy with.

May be part of the plugin config sent by the control plane?

In other words, the control plane would send the client the path to the local file that it should read?  Wouldn't that require a bunch of machinery in the control plane to allow users to set this path for each plugin instance?

How would this work in gRPC? I am definitely not sold on the idea of putting this in the bootstrap file for gRPC.

Similar to Envoy this will have to be a separate file because of the private key and the path can come from the control plane.

Let's discuss the gRPC design offline.  But I'd like to get this fleshed out before we merge this PR, so that we don't wind up in a situation where we need something that can't be provided by the API we've agreed upon.

Having a complete story for ACME wasn't originally a requirement for this PR although it was mentioned by many. Is it a requirement now?

As Matt said, I think we need to be sure we can actually support it in order to know that this API will work.

@markdroth @htuch @mattklein123 one more thing I wanted to raise: to avoid having a plugin depend on protobuf we may want to pass the config as JSON via the plugin API.
I see a couple of options:

if the plugin config coming from xDS is google.protobuf.Struct type then conversion is straightforward?
the plugin framework tries to unpack the actual type from Any and converts to JSON but that doesn't seem clean and it will need to do that for each config proto separately (a long switch statement?)

Is this an issue for the Envoy side as well? Or let's not worry about protobuf dependency?

@markdroth @htuch @mattklein123 one more thing I wanted to raise: to avoid having a plugin depend on protobuf we may want to pass the config as JSON via the plugin API.

This is not something we need to worry about, since xDS already has a general-purpose way of addressing this problem.  gRPC clients can use the envoy.config.require-any-fields-contain-struct client feature, which ensures that they will be able to convert the protobuf to JSON.

@markdroth @mattklein123 @htuch after spending some time looking into ACME I have reached the following conclusions:

it is not clear if any ACME server (Let's Encrypt specifically) can issue SPIFFE-id based certs. For the gRPC service mesh security using xDS we specifically depend on SPIFFE workload ids.

the challenge based authz mechanisms (TLS-ALPN-01 and HTTP-01) involve being able to handle incoming HTTP requests to respond to the challenges. For gRPC apps this is a bit problematic as compared to Envoy (since Envoy always has listeners enabled being a proxy). I did consider an out-of-band challenge handling component (called CLT) but that turned out to be a non-starter because of really short validity periods for the authorization key pairs.

With ACME supporting mainly FQDN certs, I see ACME as being less applicable for our xDS based gRPC service mesh use-case than for Envoy. Please advise since I seem to be stuck at this point. I am happy to look into ACME further as a separate activity but I feel this particular PR should not block on that.

With ACME supporting mainly FQDN certs, I see ACME as being less applicable for our xDS based gRPC service mesh use-case than for Envoy. Please advise since I seem to be stuck at this point. I am happy to look into ACME further as a separate activity but I feel this particular PR should not block on that.

My interest in ACME for Envoy is purely for the edge/TLS termination case. AFAICT this design will work for an ACME plugin similar to the type of config that Traefik offers. As long as we believe that is true I think it's fine?

With ACME supporting mainly FQDN certs, I see ACME as being less applicable for our xDS based gRPC service mesh use-case than for Envoy. Please advise since I seem to be stuck at this point. I am happy to look into ACME further as a separate activity but I feel this particular PR should not block on that.

My interest in ACME for Envoy is purely for the edge/TLS termination case. AFAICT this design will work for an ACME plugin similar to the type of config that Traefik offers. As long as we believe that is true I think it's fine?

I agree. To move forward I can do the following:

update my design doc with our (gRPC team's) latest findings about ACME.
address any other pending comments

And I think this PR should be ready to merge?  It looks like there is more interest in the edge case so I am looking into it and may add a para or 2 in the design doc.

And I think this PR should be ready to merge? It looks like there is more interest in the edge case so I am looking into it and may add a para or 2 in the design doc.

Yes seems fine to move forward and merge. Can you merge master to fix CI and check for merge conflicts?

And I think this PR should be ready to merge? It looks like there is more interest in the edge case so I am looking into it and may add a para or 2 in the design doc.

Yes seems fine to move forward and merge. Can you merge master to fix CI and check for merge conflicts?

will do.

And I think this PR should be ready to merge? It looks like there is more interest in the edge case so I am looking into it and may add a para or 2 in the design doc.

Yes seems fine to move forward and merge. Can you merge master to fix CI and check for merge conflicts?

@mattklein123 addressed the pending comments and rebased to master. Hopefully all okay now.

I'm still a little nervous that we haven't fleshed out the details for ACME, but if Matt is confident enough that this API will work for that when the time comes, then I'm happy with it as well.

Can you check CI?
/wait

Can you check CI?
/wait

I see the following error but I don't understand what the problem is. Any idea?
[validator][envoy/extensions/transport_sockets/tls/v3/tls.proto][msg: CertificateProvider][name] expected rules for TYPE_STRING but got TYPE_MESSAGE

Can you check CI?
/wait

fixed the issue. Hope it can be merged now


Add Ember CLI

http://reefpoints.dockyard.com/2014/06/24/introducing_ember_cli_addons.html

:+1: Would be great if Bugsnag supported this

I've released an Ember CLI addon which does this https://github.com/ivanvanderbyl/ember-bugsnag

Hey @ivanvanderbyl - thanks for releasing that! It looks great. I'm going to close this issue for now and point people to your Ember CLI addon.


A extends B relation break after wrapping by extra interface

TypeScript Version:  3.9.2

Search Terms:  subtypes, extends
Expected behavior:   {name: "age"} extends {name: string} so A extends B
Code
export type IsFlatObject<T extends object> = Extract<
  Exclude<T[keyof T],  Date | FileList>,
  any[] | object
> extends never
  ? true
  : false;

export type FieldValues = Record<string, any>;

export type FieldName<TFieldValues extends FieldValues> = IsFlatObject<
  TFieldValues
> extends true
  ? Extract<keyof TFieldValues, string>
    : string;

export type CustomElement<TFieldValues> = {
    name: FieldName<TFieldValues>;
};

type ISChild<A, B> = A extends B ? true : false

type A = CustomElement<{age: string}>  // i.e {name: "age"}
type B = CustomElement<FieldValues>   //  i.e {name: string}
type ShouldBeTrue = ISChild<{name: "age"}, {name: string}>   // type true
type ShouldBeTrueAsWell = ISChild<A, B> // type false

Output

Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

Regressed sometime between 3.3 and 3.5

is there any update on this?

So, the example as given is fixed, but not for the right reasons. Now A and B have separate aliases - A and B, respectively, so we no longer use the variance result to compare them. If you change the example to
type ShouldBeTrueAsWell = ISChild<CustomElement<{age: string}>, CustomElement<FieldValues>> // type false

the issue still repros. The root cause is that we measure all of IsFlatObject, FieldName, and CustomElement as solely covariant on their type parameters. This is incorrect. Root-cause-wise, this is a dupe of #31295. We incorrectly measure the variance of a conditional type checkType clause. While the conditional may be covariant on  a parameter within the check type (or actually bivariant, by our current relationship rules), the result of evaluating the condition, the true or false type, may be entirely unrelated, and may vary entirely differently with the type parameter in question.
The fix is simple to put together - mark conditional check type positions as Unmeasurable variance. The issue, however, is that this makes us fall back to structural comparisons when comparing many conditional instantiations which we previously shortcut (albeit potentially incorrectly) - a notable example is our own conditionalTypeDoesntSpinForever test which shows that doing that alone re-breaks the issue which #27804 (the introduction of alias measurement on conditionals directly) originally fixed, making us OOM on the test.

So, the example as given is fixed, but not for the right reasons. Now A and B have separate aliases - A and B, respectively, so we no longer use the variance result to compare them. If you change the example to
type ShouldBeTrueAsWell = ISChild<CustomElement<{age: string}>, CustomElement<FieldValues>> // type false

the issue still repros. The root cause is that we measure all of IsFlatObject, FieldName, and CustomElement as solely covariant on their type parameters. This is incorrect. Root-cause-wise, this is a dupe of #31295. We incorrectly measure the variance of a conditional type checkType clause. While the conditional may be covariant on  a parameter within the check type (or actually bivariant, by our current relationship rules), the result of evaluating the condition, the true or false type, may be entirely unrelated, and may vary entirely differently with the type parameter in question.
The fix is simple to put together - mark conditional check type positions as Unmeasurable variance. The issue, however, is that this makes us fall back to structural comparisons when comparing many conditional instantiations which we previously shortcut (albeit potentially incorrectly) - a notable example is our own conditionalTypeDoesntSpinForever test which shows that doing that alone re-breaks the issue which #27804 (the introduction of alias measurement on conditionals directly) originally fixed, making us OOM on the test.

I think the OP being fixed is sufficient to close this.


Update credential_phishing_docusign_embedded_image_lure.yml

Removing the attachment length requirement since this is supposed to be an embedded image rule.

/update-test-rules